What the developer wrote
Authored by bitromortac
bolt12: cover the amount overflow guard
The request side had a test for the offer_amount times quantity overflow, the
invoice side did not, and it was the only uncovered branch in the invoice
amount check. Verified by neutering the guard, which makes the new case accept
an invoice_amount of one against an authorized amount that wrapped to zero.
Finding F9.
https://github.com/lightningnetwork/lnd/pull/10941#discussion_r3599755895
What changed, and why it matters
This commit adds a test to ensure that when a BOLT 12 invoice is validated against a request, the multiplication of the offer amount by the requested quantity cannot overflow and wrap around to zero. Without this guard, a malicious or malformed invoice could claim an authorized amount of zero and pass validation even though the real intended amount was huge. The actual overflow guard already exists in production code; this change only adds the missing test coverage.
No immediate code change is required because the guard is already present and now tested. Reviewers should verify that the overflow guard in the non-test invoice validation code uses safe multiplication (e.g., math/bits.Mul64 or a pre-check) and consider adding fuzz tests for amount arithmetic in BOLT 12.
Security signals we found
Integer overflow / wraparound in amount calculation
Missing test coverage for security-critical branch
BOLT 12 invoice amount validation
Potential acceptance of under-funded invoice if guard were absent
Evidence from the diff
The patch introduces TestValidateInvoiceAmountOverflow in bolt12/validate_test.go. It constructs an InvoiceRequest with offer_amount=2 and invreq_quantity=2^63, which would overflow a uint64 product. It then creates an invoice with invoice_amount=0 and asserts that ValidateInvoiceAgainstRequest returns ErrAmountBelowExpected. The commit message notes that the request side already had an analogous test, the invoice side did not, and that neutering the guard causes the new test case to incorrectly accept invoice_amount=1 against an authorized amount that wrapped to zero. This confirms the existing overflow check in the invoice validation path is functional.
Changed components
bolt12/validate_test.goBOLT 12 invoice validation logic (test coverage only)Inspect captured patch +47 / −0
### bolt12/validate_test.go
@@ -2052,6 +2052,53 @@ func TestValidateInvoiceRequestAmountOverflow(t *testing.T) {
require.ErrorIs(t, writeErr, ErrAmountBelowExpected)
}
+// TestValidateInvoiceAmountOverflow is the invoice-side twin of
+// TestValidateInvoiceRequestAmountOverflow: with invreq_amount absent the
+// authorized amount is offer_amount times invreq_quantity, and that product
+// must not wrap. An unguarded multiply would truncate to zero and accept any
+// invoice_amount as "at least zero".
+func TestValidateInvoiceAmountOverflow(t *testing.T) {
+ t.Parallel()
+
+ _, pub := bobKey()
+
+ req := &InvoiceRequest{}
+ req.OfferIssuerID = tlv.SomeRecordT(
+ tlv.NewPrimitiveRecord[tlv.TlvType22](pub),
+ )
+ req.OfferAmount = tlv.SomeRecordT(
+ tlv.NewRecordT[tlv.TlvType8](TUint64(2)),
+ )
+
+ // quantity_max zero means unlimited, so the bound check does not cap
+ // the quantity below.
+ req.OfferQuantityMax = tlv.SomeRecordT(
+ tlv.NewRecordT[tlv.TlvType20](TUint64(0)),
+ )
+
+ // We request to pay 2^63 units, which would overflow the uint64 product
+ // with offer_amount(2).
+ req.InvreqQuantity = tlv.SomeRecordT(
+ tlv.NewRecordT[tlv.TlvType86](TUint64(1 << 63)),
+ )
+ req.InvreqPayerID = tlv.SomeRecordT(
+ tlv.NewPrimitiveRecord[tlv.TlvType88](pub),
+ )
+ req.InvreqMetadata = tlv.SomeRecordT(
+ tlv.NewPrimitiveRecord[tlv.TlvType0](tlv.Blob("m")),
+ )
+
+ // An invoice setting the overflow value of 0 would be accepted by an
+ // unguarded validator.
+ inv := NewInvoiceFromRequest(req)
+ inv.InvoiceAmount = tlv.SomeRecordT(
+ tlv.NewRecordT[tlv.TlvType170](TUint64(0)),
+ )
+
+ err := ValidateInvoiceAgainstRequest(inv, req)
+ require.ErrorIs(t, err, ErrAmountBelowExpected)
+}
+
// TestValidateInvoiceRequestReadChain pins the spec invreq_chain rule:
// an absent invreq_chain defaults to Bitcoin mainnet and must be
// rejected on a non-mainnet node, while a present invreq_chain thatWhy this scored 60/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.