Skip to content

Commit b98d7b8

Browse files
committed
Use saturating_mul when multiplying feerates by the fee spike buf
In theory a channel's feerate could be set to some absurd value (millions of satoshis per vB) and we'd overflow the fee spike buffer, accepting the absurd fee and ignoring our fee spike buffer check. This is harmless - the counterparty has much easier ways of bricking the channel if they want, and paying several BTC in fees is probably not the best way. Our commitment transaction and dust fee exposure logic all correctly map the `u32` to a `u64` before multiplying, making them overflow-safe. Still, its good to fix overflows because it is a remotely-reachable crash in debug builds. Reported by Jordan Mecom of Block's Security Team
1 parent 9f73a98 commit b98d7b8

2 files changed

Lines changed: 7 additions & 5 deletions

File tree

lightning/src/ln/channel.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5847,7 +5847,7 @@ impl<SP: SignerProvider> ChannelContext<SP> {
58475847
1
58485848
};
58495849
// Note that the feerate is 0 in zero-fee commitment channels, so this statement is a noop
5850-
let spiked_feerate = feerate * fee_spike_multiple;
5850+
let spiked_feerate = feerate.saturating_mul(fee_spike_multiple);
58515851
let (remote_stats, _remote_htlcs) = self
58525852
.get_next_remote_commitment_stats(
58535853
funding,
@@ -13333,7 +13333,8 @@ where
1333313333
let feerate_per_kw = if !funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() {
1333413334
// Similar to HTLC additions, require the funder to have enough funds reserved for
1333513335
// fees such that the feerate can jump without rendering the channel useless.
13336-
self.context.feerate_per_kw * FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE as u32
13336+
let spike_mul = FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE as u32;
13337+
self.context.feerate_per_kw.saturating_mul(spike_mul)
1333713338
} else {
1333813339
self.context.feerate_per_kw
1333913340
};

lightning/src/sign/tx_builder.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -336,12 +336,13 @@ fn get_available_balances(
336336
if channel_type.supports_anchor_zero_fee_commitments() { 0 } else { 1 };
337337

338338
// Note that the feerate is 0 in zero-fee commitment channels, so this statement is a noop
339-
let spiked_feerate = feerate_per_kw
340-
* if is_outbound_from_holder && !channel_type.supports_anchors_zero_fee_htlc_tx() {
339+
let spiked_feerate = feerate_per_kw.saturating_mul(
340+
if is_outbound_from_holder && !channel_type.supports_anchors_zero_fee_htlc_tx() {
341341
crate::ln::channel::FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE as u32
342342
} else {
343343
1
344-
};
344+
},
345+
);
345346

346347
let local_nondust_htlc_count = pending_htlcs
347348
.iter()

0 commit comments

Comments
 (0)