From bb4413cc657532315145a22d2901238508be40e6 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Wed, 9 Mar 2022 23:04:15 +0000 Subject: [PATCH] Make `max_total_cltv_expiry_delta` include the final CLTV This fixes an integer underflow found by the `router` fuzz target in CI. --- lightning/src/ln/functional_tests.rs | 3 ++- lightning/src/routing/router.rs | 11 +++++++---- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index ead2a8e2f..5c0f0c87b 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -6493,7 +6493,8 @@ fn test_update_add_htlc_bolt2_sender_cltv_expiry_too_high() { let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs); let _chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1000000, 0, InitFeatures::known(), InitFeatures::known()); - let (route, our_payment_hash, _, our_payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[1], vec![], 100000000, 500000001); + let (mut route, our_payment_hash, _, our_payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[1], vec![], 100000000, 0); + route.paths[0].last_mut().unwrap().cltv_expiry_delta = 500000001; unwrap_send_err!(nodes[0].node.send_payment(&route, our_payment_hash, &Some(our_payment_secret)), true, APIError::RouteError { ref err }, assert_eq!(err, &"Channel CLTV overflowed?")); } diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs index 582b6f244..48960cdec 100644 --- a/lightning/src/routing/router.rs +++ b/lightning/src/routing/router.rs @@ -670,6 +670,9 @@ where L::Target: Logger { } } } + if payment_params.max_total_cltv_expiry_delta <= final_cltv_expiry_delta { + return Err(LightningError{err: "Can't find a route where the maximum total CLTV expiry delta is below the final CLTV expiry.".to_owned(), action: ErrorAction::IgnoreError}); + } // The general routing idea is the following: // 1. Fill first/last hops communicated by the caller. @@ -866,9 +869,9 @@ where L::Target: Logger { // In order to already account for some of the privacy enhancing random CLTV // expiry delta offset we add on top later, we subtract a rough estimate // (2*MEDIAN_HOP_CLTV_EXPIRY_DELTA) here. - let max_total_cltv_expiry_delta = payment_params.max_total_cltv_expiry_delta + let max_total_cltv_expiry_delta = (payment_params.max_total_cltv_expiry_delta - final_cltv_expiry_delta) .checked_sub(2*MEDIAN_HOP_CLTV_EXPIRY_DELTA) - .unwrap_or(payment_params.max_total_cltv_expiry_delta); + .unwrap_or(payment_params.max_total_cltv_expiry_delta - final_cltv_expiry_delta); let hop_total_cltv_delta = ($next_hops_cltv_delta as u32) .checked_add($candidate.cltv_expiry_delta()) .unwrap_or(u32::max_value()); @@ -5091,7 +5094,7 @@ mod tests { .with_max_total_cltv_expiry_delta(feasible_max_total_cltv_delta); let keys_manager = test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet); let random_seed_bytes = keys_manager.get_secure_random_bytes(); - let route = get_route(&our_id, &feasible_payment_params, &network_graph, None, 100, 42, Arc::clone(&logger), &scorer, &random_seed_bytes).unwrap(); + let route = get_route(&our_id, &feasible_payment_params, &network_graph, None, 100, 0, Arc::clone(&logger), &scorer, &random_seed_bytes).unwrap(); let path = route.paths[0].iter().map(|hop| hop.short_channel_id).collect::>(); assert_ne!(path.len(), 0); @@ -5099,7 +5102,7 @@ mod tests { let fail_max_total_cltv_delta = 23; let fail_payment_params = PaymentParameters::from_node_id(nodes[6]).with_route_hints(last_hops(&nodes)) .with_max_total_cltv_expiry_delta(fail_max_total_cltv_delta); - match get_route(&our_id, &fail_payment_params, &network_graph, None, 100, 42, Arc::clone(&logger), &scorer, &random_seed_bytes) + match get_route(&our_id, &fail_payment_params, &network_graph, None, 100, 0, Arc::clone(&logger), &scorer, &random_seed_bytes) { Err(LightningError { err, .. } ) => { assert_eq!(err, "Failed to find a path to the given destination"); -- 2.39.5