Skip to content

Commit 857b4c0

Browse files
committed
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.
1 parent d28d6a5 commit 857b4c0

File tree

1 file changed

+29
-5
lines changed

1 file changed

+29
-5
lines changed

lightning/src/routing/scoring.rs

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -334,11 +334,10 @@ impl<T: Time> ChannelFailure<T> {
334334
}
335335

336336
fn decayed_penalty_msat(&self, half_life: Duration) -> u64 {
337-
let decays = self.last_updated.elapsed().as_secs().checked_div(half_life.as_secs());
338-
match decays {
339-
Some(decays) => self.undecayed_penalty_msat >> decays,
340-
None => 0,
341-
}
337+
self.last_updated.elapsed().as_secs()
338+
.checked_div(half_life.as_secs())
339+
.and_then(|decays| self.undecayed_penalty_msat.checked_shr(decays as u32))
340+
.unwrap_or(0)
342341
}
343342
}
344343

@@ -676,6 +675,31 @@ mod tests {
676675
assert_eq!(scorer.channel_penalty_msat(42, 1, Some(1), &source, &target), 1_000);
677676
}
678677

678+
#[test]
679+
fn decays_channel_failure_penalties_without_shift_overflow() {
680+
let mut scorer = Scorer::new(ScoringParameters {
681+
base_penalty_msat: 1_000,
682+
failure_penalty_msat: 512,
683+
failure_penalty_half_life: Duration::from_secs(10),
684+
overuse_penalty_start_1024th: 1024,
685+
overuse_penalty_msat_per_1024th: 0,
686+
});
687+
let source = source_node_id();
688+
let target = target_node_id();
689+
assert_eq!(scorer.channel_penalty_msat(42, 1, Some(1), &source, &target), 1_000);
690+
691+
scorer.payment_path_failed(&[], 42);
692+
assert_eq!(scorer.channel_penalty_msat(42, 1, Some(1), &source, &target), 1_512);
693+
694+
// An unchecked right shift 64 bits or more in ChannelFailure::decayed_penalty_msat would
695+
// cause an overflow.
696+
SinceEpoch::advance(Duration::from_secs(10 * 64));
697+
assert_eq!(scorer.channel_penalty_msat(42, 1, Some(1), &source, &target), 1_000);
698+
699+
SinceEpoch::advance(Duration::from_secs(10));
700+
assert_eq!(scorer.channel_penalty_msat(42, 1, Some(1), &source, &target), 1_000);
701+
}
702+
679703
#[test]
680704
fn accumulates_channel_failure_penalties_after_decay() {
681705
let mut scorer = Scorer::new(ScoringParameters {

0 commit comments

Comments
 (0)