Use `saturating_mul` when multiplying feerates by the fee spike buf
What the developer wrote
Authored by Matt Corallo
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
What changed, and why it matters
This patch fixes a potential integer overflow when calculating Lightning channel fees. If a malicious or misconfigured peer set an extremely high transaction fee rate, the software could overflow a 32-bit integer while applying a safety buffer multiplier. In release builds this would silently wrap around and ignore the safety buffer; in debug builds it would crash. The fix uses saturating multiplication so the result stays at the maximum safe value instead of overflowing. The commit notes this is not practically exploitable for theft because a peer already has easier ways to disrupt a channel, but it removes a remotely reachable debug crash.
Apply the patch. It is a low-risk defensive fix. Users running debug builds of nodes with untrusted peers are the most directly affected because the overflow would panic. No immediate incident response is required beyond normal update cadence.
Security signals we found
Integer overflow in fee calculation
Use of saturating_mul to prevent u32 wraparound/panic
Remotely reachable input (peer-provided feerate) drives the overflow
Debug-build crash (panic) possible prior to patch
Fee-spike-buffer safety check could be silently bypassed on overflow
Evidence from the diff
The commit replaces three plain u32 multiplications of feerate_per_kw (or feerate) by FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE with saturating_mul. The affected sites are in ChannelContext::check_remote_fee_and_channel_limits (channel.rs), the funding reserve calculation in ChannelContext (channel.rs), and get_available_balances in tx_builder.rs. Without the patch, an absurdly high feerate (millions of sat/vB) could overflow u32, causing the fee-spike-buffer check to be bypassed or a debug panic. Other commitment/dust-fee paths already promote to u64 before multiplying, so they were safe. The patch is defensive and partial in the sense that it only targets the identified u32 multiplication sites.
Changed components
lightning/src/ln/channel.rslightning/src/sign/tx_builder.rsInspect captured patch +7 / −5
diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index 55d4a84..b80ea76 100644
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -5847,7 +5847,7 @@ impl<SP: SignerProvider> ChannelContext<SP> {
1
};
// Note that the feerate is 0 in zero-fee commitment channels, so this statement is a noop
- let spiked_feerate = feerate * fee_spike_multiple;
+ let spiked_feerate = feerate.saturating_mul(fee_spike_multiple);
let (remote_stats, _remote_htlcs) = self
.get_next_remote_commitment_stats(
funding,
@@ -13333,7 +13333,8 @@ where
let feerate_per_kw = if !funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() {
// Similar to HTLC additions, require the funder to have enough funds reserved for
// fees such that the feerate can jump without rendering the channel useless.
- self.context.feerate_per_kw * FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE as u32
+ let spike_mul = FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE as u32;
+ self.context.feerate_per_kw.saturating_mul(spike_mul)
} else {
self.context.feerate_per_kw
};
diff --git a/lightning/src/sign/tx_builder.rs b/lightning/src/sign/tx_builder.rs
index a54f8f7..f51759d 100644
--- a/lightning/src/sign/tx_builder.rs
+++ b/lightning/src/sign/tx_builder.rs
@@ -336,12 +336,13 @@ fn get_available_balances(
if channel_type.supports_anchor_zero_fee_commitments() { 0 } else { 1 };
// Note that the feerate is 0 in zero-fee commitment channels, so this statement is a noop
- let spiked_feerate = feerate_per_kw
- * if is_outbound_from_holder && !channel_type.supports_anchors_zero_fee_htlc_tx() {
+ let spiked_feerate = feerate_per_kw.saturating_mul(
+ if is_outbound_from_holder && !channel_type.supports_anchors_zero_fee_htlc_tx() {
crate::ln::channel::FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE as u32
} else {
1
- };
+ },
+ );
let local_nondust_htlc_count = pending_htlcs
.iter()
Why this scored 53/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.