channeldb/invoice: only allow pay-addr lookup w/ no query hash

Without this check, it's possible to send a probe HTLC w/ a valid
payment address but invalid payment hash that gets settled. Because
there was no assertion that the HTLC's payment hash matches the
invoice, the link would fail when it receives an invalid preimage for
the HTLC on its commitment.
This commit is contained in:
Conner Fromknecht 2021-04-13 11:47:25 -07:00
parent 791a82878d
commit f8d201a605
No known key found for this signature in database
GPG Key ID: E7D737B67FA592C7
2 changed files with 38 additions and 1 deletions

@ -365,6 +365,36 @@ func TestAddDuplicateKeysendPayAddr(t *testing.T) {
require.Equal(t, invoice2, &dbInv2)
}
// TestFailInvoiceLookupMPPPayAddrOnly asserts that looking up a MPP invoice
// that matches _only_ by payment address fails with ErrInvoiceNotFound. This
// ensures that the HTLC's payment hash always matches the payment hash in the
// returned invoice.
func TestFailInvoiceLookupMPPPayAddrOnly(t *testing.T) {
db, cleanUp, err := MakeTestDB()
defer cleanUp()
require.NoError(t, err)
// Create and insert a random invoice.
invoice, err := randInvoice(1000)
require.NoError(t, err)
payHash := invoice.Terms.PaymentPreimage.Hash()
payAddr := invoice.Terms.PaymentAddr
_, err = db.AddInvoice(invoice, payHash)
require.NoError(t, err)
// Modify the queried payment hash to be invalid.
payHash[0] ^= 0x01
// Lookup the invoice by (invalid) payment hash and payment address. The
// lookup should fail since we require the payment hash to match for
// legacy/MPP invoices, as this guarantees that the preimage is valid
// for the given HTLC.
ref := InvoiceRefByHashAndAddr(payHash, payAddr)
_, err = db.LookupInvoice(ref)
require.Equal(t, ErrInvoiceNotFound, err)
}
// TestInvRefEquivocation asserts that retrieving or updating an invoice using
// an equivocating InvoiceRef results in ErrInvRefEquivocation.
func TestInvRefEquivocation(t *testing.T) {

@ -991,7 +991,14 @@ func fetchInvoiceNumByRef(invoiceIndex, payAddrIndex, setIDIndex kvdb.RBucket,
return invoiceNumByAddr, nil
// Return invoices by payment addr only.
case invoiceNumByAddr != nil:
//
// NOTE: We constrain this lookup to only apply if the invoice ref does
// not contain a payment hash. Legacy and MPP payments depend on the
// payment hash index to enforce that the HTLCs payment hash matches the
// payment hash for the invoice, without this check we would
// inadvertently assume the invoice contains the correct preimage for
// the HTLC, which we only enforce via the lookup by the invoice index.
case invoiceNumByAddr != nil && payHash == nil:
return invoiceNumByAddr, nil
// If we were only able to reference the invoice by hash, return the