Fix shift overflow in Scorer::channel_penalty_msat
authorJeffrey Czyz <jkczyz@gmail.com>
Wed, 1 Dec 2021 04:57:32 +0000 (22:57 -0600)
committerJeffrey Czyz <jkczyz@gmail.com>
Fri, 3 Dec 2021 20:00:52 +0000 (14:00 -0600)
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

index 2e1efa751867e3f1e6ffd1fd5f19f681765c2060..2b2839d69db82ec2640f7f24c02824b4d79b0b0b 100644 (file)
@@ -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 {