From 857b4c08a5870025a3aeb5e606972ef375d4bbea Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Tue, 30 Nov 2021 22:57:32 -0600 Subject: [PATCH] Fix shift overflow in Scorer::channel_penalty_msat An unchecked shift of more than 64 bits on u64 values causes a shift overflow panic. This may happen if a channel is penalized only once and (1) is not successfully routed through and (2) after 64 or more half life decays. Use a checked shift to prevent this from happening. --- lightning/src/routing/scoring.rs | 34 +++++++++++++++++++++++++++----- 1 file changed, 29 insertions(+), 5 deletions(-) diff --git a/lightning/src/routing/scoring.rs b/lightning/src/routing/scoring.rs index 2e1efa751..2b2839d69 100644 --- a/lightning/src/routing/scoring.rs +++ b/lightning/src/routing/scoring.rs @@ -334,11 +334,10 @@ impl ChannelFailure { } fn decayed_penalty_msat(&self, half_life: Duration) -> u64 { - let decays = self.last_updated.elapsed().as_secs().checked_div(half_life.as_secs()); - match decays { - Some(decays) => self.undecayed_penalty_msat >> decays, - None => 0, - } + self.last_updated.elapsed().as_secs() + .checked_div(half_life.as_secs()) + .and_then(|decays| self.undecayed_penalty_msat.checked_shr(decays as u32)) + .unwrap_or(0) } } @@ -676,6 +675,31 @@ mod tests { assert_eq!(scorer.channel_penalty_msat(42, 1, Some(1), &source, &target), 1_000); } + #[test] + fn decays_channel_failure_penalties_without_shift_overflow() { + let mut scorer = Scorer::new(ScoringParameters { + base_penalty_msat: 1_000, + failure_penalty_msat: 512, + failure_penalty_half_life: Duration::from_secs(10), + overuse_penalty_start_1024th: 1024, + overuse_penalty_msat_per_1024th: 0, + }); + let source = source_node_id(); + let target = target_node_id(); + assert_eq!(scorer.channel_penalty_msat(42, 1, Some(1), &source, &target), 1_000); + + scorer.payment_path_failed(&[], 42); + assert_eq!(scorer.channel_penalty_msat(42, 1, Some(1), &source, &target), 1_512); + + // An unchecked right shift 64 bits or more in ChannelFailure::decayed_penalty_msat would + // cause an overflow. + SinceEpoch::advance(Duration::from_secs(10 * 64)); + assert_eq!(scorer.channel_penalty_msat(42, 1, Some(1), &source, &target), 1_000); + + SinceEpoch::advance(Duration::from_secs(10)); + assert_eq!(scorer.channel_penalty_msat(42, 1, Some(1), &source, &target), 1_000); + } + #[test] fn accumulates_channel_failure_penalties_after_decay() { let mut scorer = Scorer::new(ScoringParameters { -- 2.39.5