common/amount: prevent scaling with invalid factors in amount scale functions
What the developer wrote
Authored by Chandra Pratap
common/amount: prevent scaling with invalid factors in amount scale functions
Changelog-Fixed: Add checks in `amount_msat_scale` and
`amount_sat_scale` to return false when the scaling factor
is -NaN or negative.
This is important as the scaling factor may come from external
sources like wire, which can cause runtime errors if not handled
properly.
What changed, and why it matters
This commit adds safety checks to two internal functions that multiply cryptocurrency amounts by a scaling factor. Previously, if the scaling factor was a malformed 'not-a-number' value or a negative number, the function could produce undefined or nonsensical results. Now the functions reject those inputs outright. The commit message says the factor can come from network data, so this is a hardening fix against bad or malicious input.
Review all callers of amount_msat_scale() and amount_sat_scale() to ensure they handle a false return value safely, especially code paths that parse scale factors from peer messages. Consider adding unit tests covering NaN, negative, zero, very large, and subnormal scale values.
Security signals we found
Input validation added to functions consuming externally supplied scaling factors
NaN and negative scale values now rejected instead of producing invalid double results
Commit message explicitly notes the scaling factor may come from the wire
Functions are marked WARN_UNUSED_RESULT, encouraging callers to check return value
Evidence from the diff
In common/amount.c, amount_msat_scale() and amount_sat_scale() now test if (scale != scale || scale < 0) return false; before performing the multiplication. scale != scale is the standard C idiom for detecting NaN, and the < 0 check rejects negative multipliers. Both functions return bool and already had WARN_UNUSED_RESULT annotations. The patch is minimal (+6 lines) and only adds guard clauses; it does not change callers or add tests.
Changed components
common/amount.camount_msat_scale()amount_sat_scale()Inspect captured patch +6 / −0
diff --git a/common/amount.c b/common/amount.c
index 1eae69ce..0f186736 100644
--- a/common/amount.c
+++ b/common/amount.c
@@ -336,6 +336,9 @@ WARN_UNUSED_RESULT bool amount_msat_scale(struct amount_msat *val,
struct amount_msat msat,
double scale)
{
+ if (scale != scale || scale < 0)
+ return false;
+
double scaled = msat.millisatoshis * scale;
/* If mantissa is < 64 bits, a naive "if (scaled >
@@ -350,6 +353,9 @@ WARN_UNUSED_RESULT bool amount_sat_scale(struct amount_sat *val,
struct amount_sat sat,
double scale)
{
+ if (scale != scale || scale < 0)
+ return false;
+
double scaled = sat.satoshis * scale;
/* If mantissa is < 64 bits, a naive "if (scaled >
Why this scored 51/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.