Skip to content

Commit 10fe63d

Browse files
committed
Add onion failure packet length check to prevent out of bounds error
Fixes an oversight in the refactor in commit ea0f099 when moving the decoding of the packet.
1 parent 030a784 commit 10fe63d

File tree

2 files changed

+30
-0
lines changed

2 files changed

+30
-0
lines changed

lightning/src/ln/onion_route_tests.rs

+7
Original file line numberDiff line numberDiff line change
@@ -692,6 +692,13 @@ fn test_onion_failure() {
692692
}, || nodes[2].node.fail_htlc_backwards(&payment_hash), false, None,
693693
Some(NetworkUpdate::NodeFailure { node_id: route.paths[0].hops[1].pubkey, is_permanent: true }),
694694
Some(channels[1].0.contents.short_channel_id), None);
695+
696+
run_onion_failure_test_with_fail_intercept("bogus err packet that is too short for an hmac", 200, &nodes,
697+
&route, &payment_hash, &payment_secret, |_msg| {}, |msg| {
698+
msg.reason = vec![1, 2, 3];
699+
}, || nodes[2].node.fail_htlc_backwards(&payment_hash), false, None,
700+
None, None, None);
701+
695702
run_onion_failure_test_with_fail_intercept("0-length channel update in intermediate node UPDATE onion failure",
696703
100, &nodes, &route, &payment_hash, &payment_secret, |msg| {
697704
msg.amount_msat -= 1;

lightning/src/ln/onion_utils.rs

+23
Original file line numberDiff line numberDiff line change
@@ -991,6 +991,29 @@ where
991991
_ => unreachable!(),
992992
};
993993

994+
// Check that there is at least enough data for an hmac, otherwise none of the checking that we may do makes sense.
995+
// Also prevent slice out of bounds further down.
996+
if encrypted_packet.data.len() < 32 {
997+
log_warn!(
998+
logger,
999+
"Non-attributable failure encountered on route {}",
1000+
path.hops.iter().map(|h| h.pubkey.to_string()).collect::<Vec<_>>().join("->")
1001+
);
1002+
1003+
// Signal that we failed permanently. Without a valid hmac, we can't identify the failing node and we can't
1004+
// apply a penalty. Therefore there is nothing more we can do other than failing the payment.
1005+
return DecodedOnionFailure {
1006+
network_update: None,
1007+
short_channel_id: None,
1008+
payment_failed_permanently: true,
1009+
failed_within_blinded_path: false,
1010+
#[cfg(any(test, feature = "_test_utils"))]
1011+
onion_error_code: None,
1012+
#[cfg(any(test, feature = "_test_utils"))]
1013+
onion_error_data: None,
1014+
};
1015+
}
1016+
9941017
// Learnings from the HTLC failure to inform future payment retries and scoring.
9951018
struct FailureLearnings {
9961019
network_update: Option<NetworkUpdate>,

0 commit comments

Comments
 (0)