Handle overflowing route-hint fee aggregates
What changed, and why it matters
This patch fixes a crash bug in the Lightning routing code. When a user disabled the normal cap on routing fees, a malicious or specially crafted payment invoice containing route hints with extremely high proportional fees could cause an internal arithmetic overflow. Previously this overflow made the program panic; now the code detects the overflow and treats that route as unusable, so routing fails cleanly instead of crashing.
Apply the patch. It is a targeted defensive fix with a regression test and low risk of functional regression because it only changes behavior on the previously panicking overflow path. Users who do not disable the routing fee cap are not affected by the panic, but should still update as part of normal maintenance.
Security signals we found
Integer overflow in fee aggregation previously caused panic (denial of service)
Trigger requires attacker-controlled invoice route hints with large proportional fees
Only exploitable when the routing fee cap is disabled by the payer
Patch converts panic path into graceful route-failure path
Regression test added: aggregated_prop_fee_overflow_fails_route
Evidence from the diff
In rust-lightning’s router, PaymentPath::max_final_value_msat calls compute_aggregated_base_prop_fee to sum base/proportional fees of downstream hops. That helper returns an Err on overflow, but the caller previously unwrapped it, causing a panic when a crafted route hint had proportional_millionths near u32::MAX on multiple hops and the payer set max_total_routing_fee_msat: None (disabling the fee cap). The patch changes max_final_value_msat to return Result<(usize,u64), usize>, propagating the overflow as an index of a hop to discard. The caller in get_route then marks that candidate’s liquidity exhausted and continues path collection, failing routing cleanly. A regression test confirms an error is returned instead of a panic.
Changed components
lightning/src/routing/router.rsPaymentPath::max_final_value_msatget_route path-selection loopblinded_path::payment::compute_aggregated_base_prop_feeInspect captured patch +90 / −4
diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs
index 4ac8987..be33fa4 100644
--- a/lightning/src/routing/router.rs
+++ b/lightning/src/routing/router.rs
@@ -2421,10 +2421,12 @@ impl<'a> PaymentPath<'a> {
/// contribution this path can make to the final value of the payment.
/// May be slightly lower than the actual max due to rounding errors when aggregating fees
/// along the path.
+ /// Returns an error with the index of a later hop to discard if the following hops' aggregate
+ /// fees overflow.
#[rustfmt::skip]
fn max_final_value_msat(
&self, used_liquidities: &HashMap<CandidateHopId, u64>, channel_saturation_pow_half: u8
- ) -> (usize, u64) {
+ ) -> Result<(usize, u64), usize> {
let mut max_path_contribution = (0, u64::MAX);
for (idx, (hop, _)) in self.hops.iter().enumerate() {
let hop_effective_capacity_msat = hop.candidate.effective_capacity();
@@ -2440,7 +2442,8 @@ impl<'a> PaymentPath<'a> {
// Aggregate the fees of the hops that come after this one, and use those fees to compute the
// maximum amount that this hop can contribute to the final value received by the payee.
let (next_hops_aggregated_base, next_hops_aggregated_prop) =
- crate::blinded_path::payment::compute_aggregated_base_prop_fee(next_hops_feerates_iter).unwrap();
+ crate::blinded_path::payment::compute_aggregated_base_prop_fee(next_hops_feerates_iter)
+ .map_err(|_| idx + 1)?;
// floor(((hop_max_msat - agg_base) * 1_000_000) / (1_000_000 + agg_prop))
let hop_max_final_value_contribution = (hop_max_msat as u128)
@@ -2457,7 +2460,19 @@ impl<'a> PaymentPath<'a> {
} else { debug_assert!(false); }
}
- max_path_contribution
+ Ok(max_path_contribution)
+ }
+}
+
+fn mark_candidate_liquidity_exhausted(
+ used_liquidities: &mut HashMap<CandidateHopId, u64>, candidate: &CandidateRouteHop,
+) {
+ let exhausted = u64::max_value();
+ if let Some(scid) = candidate.short_channel_id() {
+ *used_liquidities.entry(CandidateHopId::Clear((scid, false))).or_default() = exhausted;
+ *used_liquidities.entry(CandidateHopId::Clear((scid, true))).or_default() = exhausted;
+ } else {
+ *used_liquidities.entry(candidate.id()).or_default() = exhausted;
}
}
@@ -3637,7 +3652,17 @@ pub(crate) fn get_route<L: Logger, S: ScoreLookUp>(
// underpaid htlc_minimum_msat with fees.
debug_assert_eq!(payment_path.get_value_msat(), value_contribution_msat);
let (lowest_value_contrib_hop, max_path_contribution_msat) =
- payment_path.max_final_value_msat(&used_liquidities, channel_saturation_pow_half);
+ match payment_path.max_final_value_msat(&used_liquidities, channel_saturation_pow_half) {
+ Ok(contribution) => contribution,
+ Err(candidate_idx_to_skip) => {
+ let candidate = &payment_path.hops[candidate_idx_to_skip].0.candidate;
+ log_trace!(logger,
+ "Ignoring path because aggregate fees including hop {} overflow.",
+ LoggedCandidateHop(candidate));
+ mark_candidate_liquidity_exhausted(&mut used_liquidities, candidate);
+ continue 'paths_collection;
+ }
+ };
let desired_value_contribution = cmp::min(max_path_contribution_msat, final_value_msat);
value_contribution_msat = payment_path.update_value_and_recompute_fees(desired_value_contribution);
@@ -9332,6 +9357,67 @@ mod tests {
assert_eq!(route.paths[0].hops[0].short_channel_id, 44);
}
+ #[test]
+ fn aggregated_prop_fee_overflow_fails_route() {
+ // If the fee cap is disabled, we may consider invoice hints with very large
+ // proportional fees. Aggregating those fees can overflow, in which case we should fail
+ // routing cleanly rather than panic.
+ let secp_ctx = Secp256k1::new();
+ let logger = Arc::new(ln_test_utils::TestLogger::new());
+ let network_graph = Arc::new(NetworkGraph::new(Network::Testnet, Arc::clone(&logger)));
+ let scorer = ln_test_utils::TestScorer::new();
+ let random_seed_bytes = [42; 32];
+ let config = UserConfig::default();
+
+ let (_, our_node_id, _, nodes) = get_nodes(&secp_ctx);
+ let route_hint = RouteHint(vec![
+ RouteHintHop {
+ src_node_id: nodes[0],
+ short_channel_id: 100,
+ fees: RoutingFees { base_msat: 0, proportional_millionths: u32::MAX },
+ cltv_expiry_delta: 10,
+ htlc_minimum_msat: None,
+ htlc_maximum_msat: None,
+ },
+ RouteHintHop {
+ src_node_id: nodes[1],
+ short_channel_id: 101,
+ fees: RoutingFees { base_msat: 0, proportional_millionths: u32::MAX },
+ cltv_expiry_delta: 10,
+ htlc_minimum_msat: None,
+ htlc_maximum_msat: None,
+ },
+ ]);
+
+ let payment_params = PaymentParameters::from_node_id(nodes[2], 42)
+ .with_route_hints(vec![route_hint])
+ .unwrap()
+ .with_bolt11_features(channelmanager::provided_bolt11_invoice_features(&config))
+ .unwrap();
+ let first_hops = [get_channel_details(
+ Some(1),
+ nodes[0],
+ channelmanager::provided_init_features(&config),
+ 100_000_000,
+ )];
+ let route_params = RouteParameters {
+ payment_params,
+ final_value_msat: 1,
+ max_total_routing_fee_msat: None,
+ };
+ let route = get_route(
+ &our_node_id,
+ &route_params,
+ &network_graph.read_only(),
+ Some(&first_hops.iter().collect::<Vec<_>>()),
+ Arc::clone(&logger),
+ &scorer,
+ &Default::default(),
+ &random_seed_bytes,
+ );
+ assert!(route.is_err());
+ }
+
#[test]
fn prefers_paths_by_cost_amt_ratio() {
// Previously, we preferred paths during MPP selection based on their absolute cost, rather
Why this scored 62/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.