From 80ba9acd775852d1bfa6166a7ec64e8625586a6d Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Fri, 8 Dec 2023 17:23:01 -0500 Subject: [PATCH] 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." --- lightning/src/ln/onion_payment.rs | 34 ++++++++++++++++++++++++++++++- lightning/src/ln/onion_utils.rs | 4 ++-- 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/lightning/src/ln/onion_payment.rs b/lightning/src/ln/onion_payment.rs index 07c08d579..7c0d32a52 100644 --- a/lightning/src/ln/onion_payment.rs +++ b/lightning/src/ln/onion_payment.rs @@ -449,7 +449,7 @@ pub(super) fn check_incoming_htlc_cltv( mod tests { use bitcoin::hashes::Hash; use bitcoin::hashes::sha256::Hash as Sha256; - use bitcoin::secp256k1::{PublicKey, SecretKey}; + use bitcoin::secp256k1::{PublicKey, Secp256k1, SecretKey}; use crate::ln::{PaymentPreimage, PaymentHash, PaymentSecret}; use crate::ln::ChannelId; use crate::ln::channelmanager::RecipientOnionFields; @@ -459,6 +459,38 @@ mod tests { use crate::routing::router::{Path, RouteHop}; use crate::util::test_utils; + #[test] + fn fail_construct_onion_on_too_big_payloads() { + // 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." + let secp_ctx = Secp256k1::new(); + let bob = crate::sign::KeysManager::new(&[2; 32], 42, 42); + let bob_pk = PublicKey::from_secret_key(&secp_ctx, &bob.get_node_secret_key()); + let charlie = crate::sign::KeysManager::new(&[3; 32], 42, 42); + let charlie_pk = PublicKey::from_secret_key(&secp_ctx, &charlie.get_node_secret_key()); + + let ( + session_priv, total_amt_msat, cur_height, mut recipient_onion, keysend_preimage, payment_hash, + prng_seed, hops, .. + ) = payment_onion_args(bob_pk, charlie_pk); + + // Ensure the onion will not fit all the payloads by adding a large custom TLV. + recipient_onion.custom_tlvs.push((13377331, vec![0; 1156])); + + let path = Path { hops, blinded_tail: None, }; + let onion_keys = super::onion_utils::construct_onion_keys(&secp_ctx, &path, &session_priv).unwrap(); + let (onion_payloads, ..) = super::onion_utils::build_onion_payloads( + &path, total_amt_msat, recipient_onion, cur_height + 1, &Some(keysend_preimage) + ).unwrap(); + + assert!(super::onion_utils::construct_onion_packet( + onion_payloads, onion_keys, prng_seed, &payment_hash + ).is_err()); + } + #[test] fn test_peel_payment_onion() { use super::*; diff --git a/lightning/src/ln/onion_utils.rs b/lightning/src/ln/onion_utils.rs index a9f8074f5..ea906c9e3 100644 --- a/lightning/src/ln/onion_utils.rs +++ b/lightning/src/ln/onion_utils.rs @@ -323,8 +323,6 @@ fn construct_onion_packet_with_init_noise( let mut pos = 0; for (i, (payload, keys)) in payloads.iter().zip(onion_keys.iter()).enumerate() { - if i == payloads.len() - 1 { break; } - let mut chacha = ChaCha20::new(&keys.rho, &[0u8; 8]); for _ in 0..(packet_data.len() - pos) { // TODO: Batch this. let mut dummy = [0; 1]; @@ -338,6 +336,8 @@ fn construct_onion_packet_with_init_noise( return Err(()); } + if i == payloads.len() - 1 { break; } + res.resize(pos, 0u8); chacha.process_in_place(&mut res); } -- 2.39.5