common: saturate marginal_feerate() instead of overflowing
What the developer wrote
Authored by Matt Morehouse
common: saturate marginal_feerate() instead of overflowing
marginal_feerate() computed current_feerate * 1.1 as a double and
converted the result back to u32. Since current_feerate is chosen by
the peer in open_channel or update_fee, they could choose an absurdly
high value that overflows u32 after the computation. UBSan reports:
common/fee_states.c:179:10: runtime error: 4.72446e+09 is outside the range of representable values of type 'unsigned int'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior common/fee_states.c:179:10
Do the arithmetic with u64 and saturate at UINT32_MAX to avoid the
undefined behavior.
Found by fuzzing with smite.
Changelog-Fixed: JSON-RPC: `listpeerchannels` no longer derives `receivable_msat` from an overflowed fee estimate when the peer sets an absurd `feerate_per_kw`.
What changed, and why it matters
This commit fixes a numeric overflow bug in Core Lightning's fee calculation. A peer could supply an extremely high transaction fee rate that, when increased by 10%, produced a value too large for the program's internal storage. That triggered undefined behavior (a C programming error) and could cause incorrect values, such as a distorted 'receivable_msat' shown by listpeerchannels. The fix performs the math in a larger integer type and caps the result at the maximum allowed value.
Apply the patch and ensure builds with UBSan/ASan continue to pass. Review other fee-rate arithmetic for similar double-to-integer conversions or missing saturation. The issue was found by fuzzing; consider extending fuzzing coverage for peer-controlled numeric inputs.
Security signals we found
UndefinedBehaviorSanitizer-reported double-to-u32 overflow
Peer-controlled input (feerate_per_kw) used in arithmetic without bounds checking
Downstream RPC value (listpeerchannels receivable_msat) derived from overflowed fee estimate
Fix uses widened u64 arithmetic and explicit saturation
Evidence from the diff
marginal_feerate() in common/fee_states.c previously computed current_feerate * 1.1 using double and cast back to u32. Because current_feerate is attacker-controlled via open_channel/update_fee, a sufficiently large value caused a double-to-u32 conversion outside the representable range, reported by UBSan as undefined behavior. The patch computes ((u64)current_feerate * 11) / 10 and saturates at UINT32_MAX. A regression test verifies the boundary around 3904515723 and UINT32_MAX.
Changed components
common/fee_states.cmarginal_feerate()common/test/run-marginal_feerate.cJSON-RPC listpeerchannels receivable_msat calculationInspect captured patch +13 / −2
### common/fee_states.c
@@ -175,8 +175,12 @@ u32 marginal_feerate(u32 current_feerate)
/* This could happen in future if we celebrate sub-sat summer! */
if (current_feerate < minfeerate)
current_feerate = minfeerate;
- if (current_feerate > maxfeerate)
- return current_feerate * 1.1;
+ if (current_feerate > maxfeerate) {
+ u64 marginal = ((u64)current_feerate * 11) / 10;
+ if (marginal > UINT32_MAX)
+ return UINT32_MAX;
+ return marginal;
+ }
/* min gives 1, max gives 0.1 */
double proportion = 1.0 - ((double)current_feerate - minfeerate) / (maxfeerate - minfeerate) * 0.9;
### common/test/run-marginal_feerate.c
@@ -127,5 +127,12 @@ int main(int argc, char *argv[])
u32 half = (45000 + 253)/2;
assert(marginal_feerate(half) == (u32)(half * 1.55));
+ /* These values straddle the case where adding 10% exceeds UINT32_MAX.
+ * No overflow should occur. */
+ assert(marginal_feerate(3904515722) == UINT32_MAX - 1);
+ assert(marginal_feerate(3904515723) == UINT32_MAX);
+ assert(marginal_feerate(3904515724) == UINT32_MAX);
+ assert(marginal_feerate(UINT32_MAX) == UINT32_MAX);
+
common_shutdown();
}Why this scored 62/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.