Skip to content

Commit

Permalink
channeldb/invoice: only allow pay-addr lookup w/ no query hash
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
cfromknecht committed Apr 13, 2021
1 parent 791a828 commit f8d201a
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 1 deletion.
30 changes: 30 additions & 0 deletions channeldb/invoice_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
9 changes: 8 additions & 1 deletion channeldb/invoices.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit f8d201a

Please sign in to comment.