From: Jeffrey Czyz <jkczyz@gmail.com>
Date: Wed, 1 Dec 2021 04:57:32 +0000 (-0600)
Subject: Fix shift overflow in Scorer::channel_penalty_msat
X-Git-Tag: v0.0.104~16^2
X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=commitdiff_plain;h=857b4c08a5870025a3aeb5e606972ef375d4bbea;p=rust-lightning

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.
---

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<T: Time> ChannelFailure<T> {
 	}
 
 	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 {