Skip to content

Commit 62c1205

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 62c1205

File tree

2 files changed

+26
-0
lines changed

2 files changed

+26
-0
lines changed

lightning/src/ln/onion_route_tests.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -692,6 +692,11 @@ 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+
run_onion_failure_test_with_fail_intercept("bogus err packet that is too short for an hmac", 200, &nodes,
696+
&route, &payment_hash, &payment_secret, |_msg| {}, |msg| {
697+
msg.reason = vec![1, 2, 3];
698+
}, || nodes[2].node.fail_htlc_backwards(&payment_hash), false, None,
699+
None, None, None);
695700
run_onion_failure_test_with_fail_intercept("0-length channel update in intermediate node UPDATE onion failure",
696701
100, &nodes, &route, &payment_hash, &payment_secret, |msg| {
697702
msg.amount_msat -= 1;

lightning/src/ln/onion_utils.rs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -991,6 +991,27 @@ 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+
return DecodedOnionFailure {
1004+
network_update: None,
1005+
short_channel_id: None,
1006+
payment_failed_permanently: true,
1007+
failed_within_blinded_path: false,
1008+
#[cfg(any(test, feature = "_test_utils"))]
1009+
onion_error_code: None,
1010+
#[cfg(any(test, feature = "_test_utils"))]
1011+
onion_error_data: None,
1012+
};
1013+
}
1014+
9941015
// Learnings from the HTLC failure to inform future payment retries and scoring.
9951016
struct FailureLearnings {
9961017
network_update: Option<NetworkUpdate>,

0 commit comments

Comments
 (0)