Skip to content

Commit df9ceed

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 df9ceed

File tree

1 file changed

+26
-8
lines changed

1 file changed

+26
-8
lines changed

lightning/src/ln/onion_utils.rs

Lines changed: 26 additions & 8 deletions
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,9 +333,12 @@ 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,
336+
let final_value_msat_with_overpay_buffer = core::cmp::min(
337+
0x1000_0000,
338+
core::cmp::max(
339+
route_params.final_value_msat.saturating_mul(OVERPAY_ESTIMATE_MULTIPLER),
340+
MIN_FINAL_VALUE_ESTIMATE_WITH_OVERPAY,
341+
)
339342
);
340343

341344
let blinded_tail_opt = route_params
@@ -357,7 +360,7 @@ pub(crate) fn set_max_path_length(
357360
short_channel_id: 42,
358361
channel_features: ChannelFeatures::empty(),
359362
fee_msat: final_value_msat_with_overpay_buffer,
360-
cltv_expiry_delta: route_params.payment_params.max_total_cltv_expiry_delta,
363+
cltv_expiry_delta: core::cmp::min(route_params.payment_params.max_total_cltv_expiry_delta, 0x1000_0000),
361364
maybe_announced_channel: false,
362365
};
363366
let mut num_reserved_bytes: usize = 0;
@@ -1280,7 +1283,7 @@ fn decode_next_hop<T, R: ReadableArgs<T>, N: NextPacketBytes>(
12801283
mod tests {
12811284
use crate::io;
12821285
use crate::ln::msgs;
1283-
use crate::routing::router::{Path, Route, RouteHop};
1286+
use crate::routing::router::{PaymentParameters, Path, Route, RouteHop};
12841287
use crate::types::features::{ChannelFeatures, NodeFeatures};
12851288
use crate::types::payment::PaymentHash;
12861289
use crate::util::ser::{VecWriter, Writeable, Writer};
@@ -1292,7 +1295,7 @@ mod tests {
12921295
use bitcoin::secp256k1::Secp256k1;
12931296
use bitcoin::secp256k1::{PublicKey, SecretKey};
12941297

1295-
use super::OnionKeys;
1298+
use super::*;
12961299

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

0 commit comments

Comments
 (0)