Skip to content

Commit 80ba9ac

Browse files
Error if onion payloads exceed max length on packet construction.
Ensure that if we call construct_onion_packet and friends where payloads are too large for the allotted packet length, we'll fail to construct. Previously, senders would happily construct invalid packets by array-shifting the final node's HMAC out of the packet when adding an intermediate onion layer, causing the receiver to error with "final payload provided for us as an intermediate node."
1 parent e9bd893 commit 80ba9ac

File tree

2 files changed

+35
-3
lines changed

2 files changed

+35
-3
lines changed

lightning/src/ln/onion_payment.rs

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -449,7 +449,7 @@ pub(super) fn check_incoming_htlc_cltv(
449449
mod tests {
450450
use bitcoin::hashes::Hash;
451451
use bitcoin::hashes::sha256::Hash as Sha256;
452-
use bitcoin::secp256k1::{PublicKey, SecretKey};
452+
use bitcoin::secp256k1::{PublicKey, Secp256k1, SecretKey};
453453
use crate::ln::{PaymentPreimage, PaymentHash, PaymentSecret};
454454
use crate::ln::ChannelId;
455455
use crate::ln::channelmanager::RecipientOnionFields;
@@ -459,6 +459,38 @@ mod tests {
459459
use crate::routing::router::{Path, RouteHop};
460460
use crate::util::test_utils;
461461

462+
#[test]
463+
fn fail_construct_onion_on_too_big_payloads() {
464+
// Ensure that if we call `construct_onion_packet` and friends where payloads are too large for
465+
// the allotted packet length, we'll fail to construct. Previously, senders would happily
466+
// construct invalid packets by array-shifting the final node's HMAC out of the packet when
467+
// adding an intermediate onion layer, causing the receiver to error with "final payload
468+
// provided for us as an intermediate node."
469+
let secp_ctx = Secp256k1::new();
470+
let bob = crate::sign::KeysManager::new(&[2; 32], 42, 42);
471+
let bob_pk = PublicKey::from_secret_key(&secp_ctx, &bob.get_node_secret_key());
472+
let charlie = crate::sign::KeysManager::new(&[3; 32], 42, 42);
473+
let charlie_pk = PublicKey::from_secret_key(&secp_ctx, &charlie.get_node_secret_key());
474+
475+
let (
476+
session_priv, total_amt_msat, cur_height, mut recipient_onion, keysend_preimage, payment_hash,
477+
prng_seed, hops, ..
478+
) = payment_onion_args(bob_pk, charlie_pk);
479+
480+
// Ensure the onion will not fit all the payloads by adding a large custom TLV.
481+
recipient_onion.custom_tlvs.push((13377331, vec![0; 1156]));
482+
483+
let path = Path { hops, blinded_tail: None, };
484+
let onion_keys = super::onion_utils::construct_onion_keys(&secp_ctx, &path, &session_priv).unwrap();
485+
let (onion_payloads, ..) = super::onion_utils::build_onion_payloads(
486+
&path, total_amt_msat, recipient_onion, cur_height + 1, &Some(keysend_preimage)
487+
).unwrap();
488+
489+
assert!(super::onion_utils::construct_onion_packet(
490+
onion_payloads, onion_keys, prng_seed, &payment_hash
491+
).is_err());
492+
}
493+
462494
#[test]
463495
fn test_peel_payment_onion() {
464496
use super::*;

lightning/src/ln/onion_utils.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -323,8 +323,6 @@ fn construct_onion_packet_with_init_noise<HD: Writeable, P: Packet>(
323323

324324
let mut pos = 0;
325325
for (i, (payload, keys)) in payloads.iter().zip(onion_keys.iter()).enumerate() {
326-
if i == payloads.len() - 1 { break; }
327-
328326
let mut chacha = ChaCha20::new(&keys.rho, &[0u8; 8]);
329327
for _ in 0..(packet_data.len() - pos) { // TODO: Batch this.
330328
let mut dummy = [0; 1];
@@ -338,6 +336,8 @@ fn construct_onion_packet_with_init_noise<HD: Writeable, P: Packet>(
338336
return Err(());
339337
}
340338

339+
if i == payloads.len() - 1 { break; }
340+
341341
res.resize(pos, 0u8);
342342
chacha.process_in_place(&mut res);
343343
}

0 commit comments

Comments
 (0)