Skip to content

Commit 3e88b32

Browse files
committed
Fix max-value overflows in set_max_path_length
When either the amount or the `max_total_cltv_expiry_delta` are set to max-value, `set_max_path_length` can trigger overflows in `build_onion_payloads_callback`, leading to debug-panics.
1 parent 463e432 commit 3e88b32

File tree

1 file changed

+26
-9
lines changed

1 file changed

+26
-9
lines changed

lightning/src/ln/onion_utils.rs

+26-9
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,7 @@ where
239239
// the intended recipient).
240240
let value_msat = if cur_value_msat == 0 { hop.fee_msat } else { cur_value_msat };
241241
let cltv = if cur_cltv == starting_htlc_offset {
242-
hop.cltv_expiry_delta + starting_htlc_offset
242+
hop.cltv_expiry_delta.saturating_add(starting_htlc_offset)
243243
} else {
244244
cur_cltv
245245
};
@@ -307,7 +307,7 @@ where
307307
if cur_value_msat >= 21000000 * 100000000 * 1000 {
308308
return Err(APIError::InvalidRoute { err: "Channel fees overflowed?".to_owned() });
309309
}
310-
cur_cltv += hop.cltv_expiry_delta as u32;
310+
cur_cltv = cur_cltv.saturating_add(hop.cltv_expiry_delta as u32);
311311
if cur_cltv >= 500000000 {
312312
return Err(APIError::InvalidRoute { err: "Channel CLTV overflowed?".to_owned() });
313313
}
@@ -333,10 +333,10 @@ pub(crate) fn set_max_path_length(
333333
.saturating_add(PAYLOAD_HMAC_LEN);
334334

335335
const OVERPAY_ESTIMATE_MULTIPLER: u64 = 3;
336-
let final_value_msat_with_overpay_buffer = core::cmp::max(
337-
route_params.final_value_msat.saturating_mul(OVERPAY_ESTIMATE_MULTIPLER),
338-
MIN_FINAL_VALUE_ESTIMATE_WITH_OVERPAY,
339-
);
336+
let final_value_msat_with_overpay_buffer = route_params
337+
.final_value_msat
338+
.saturating_mul(OVERPAY_ESTIMATE_MULTIPLER)
339+
.clamp(MIN_FINAL_VALUE_ESTIMATE_WITH_OVERPAY, 0x1000_0000);
340340

341341
let blinded_tail_opt = route_params
342342
.payment_params
@@ -351,13 +351,15 @@ pub(crate) fn set_max_path_length(
351351
excess_final_cltv_expiry_delta: 0,
352352
});
353353

354+
let cltv_expiry_delta =
355+
core::cmp::min(route_params.payment_params.max_total_cltv_expiry_delta, 0x1000_0000);
354356
let unblinded_route_hop = RouteHop {
355357
pubkey: PublicKey::from_slice(&[2; 33]).unwrap(),
356358
node_features: NodeFeatures::empty(),
357359
short_channel_id: 42,
358360
channel_features: ChannelFeatures::empty(),
359361
fee_msat: final_value_msat_with_overpay_buffer,
360-
cltv_expiry_delta: route_params.payment_params.max_total_cltv_expiry_delta,
362+
cltv_expiry_delta,
361363
maybe_announced_channel: false,
362364
};
363365
let mut num_reserved_bytes: usize = 0;
@@ -1280,7 +1282,7 @@ fn decode_next_hop<T, R: ReadableArgs<T>, N: NextPacketBytes>(
12801282
mod tests {
12811283
use crate::io;
12821284
use crate::ln::msgs;
1283-
use crate::routing::router::{Path, Route, RouteHop};
1285+
use crate::routing::router::{Path, PaymentParameters, Route, RouteHop};
12841286
use crate::types::features::{ChannelFeatures, NodeFeatures};
12851287
use crate::types::payment::PaymentHash;
12861288
use crate::util::ser::{VecWriter, Writeable, Writer};
@@ -1292,7 +1294,7 @@ mod tests {
12921294
use bitcoin::secp256k1::Secp256k1;
12931295
use bitcoin::secp256k1::{PublicKey, SecretKey};
12941296

1295-
use super::OnionKeys;
1297+
use super::*;
12961298

12971299
fn get_test_session_key() -> SecretKey {
12981300
let hex = "4141414141414141414141414141414141414141414141414141414141414141";
@@ -1607,4 +1609,19 @@ mod tests {
16071609
writer.write_all(&self.data[..])
16081610
}
16091611
}
1612+
1613+
#[test]
1614+
fn max_length_with_no_cltv_limit() {
1615+
// While users generally shouldn't do this, we shouldn't overflow when
1616+
// `max_total_cltv_expiry_delta` is `u32::MAX`.
1617+
let recipient = PublicKey::from_slice(&[2; 33]).unwrap();
1618+
let mut route_params = RouteParameters {
1619+
payment_params: PaymentParameters::for_keysend(recipient, u32::MAX, true),
1620+
final_value_msat: u64::MAX,
1621+
max_total_routing_fee_msat: Some(u64::MAX),
1622+
};
1623+
route_params.payment_params.max_total_cltv_expiry_delta = u32::MAX;
1624+
let recipient_onion = RecipientOnionFields::spontaneous_empty();
1625+
set_max_path_length(&mut route_params, &recipient_onion, None, None, 42).unwrap();
1626+
}
16101627
}

0 commit comments

Comments
 (0)