akovalenko/lnd.xprv
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

invoices: always check htlc amt with invoice amount

Browse Source 
Previously a check was made for accepted and settled invoices against
the paid amount. This opens up a probe vector where an attacker can pay
to an invoice with an amt that is higher than the invoice amount and
find out if the invoice is already paid or not.
This commit is contained in:
Joost Jager 2019-08-08 16:25:25 +02:00
parent 762609a169
commit 43bad4af9f
No known key found for this signature in database
GPG Key ID: A61B9D4C393C59C7
2 changed files with 11 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
invoices/invoiceregistry.go
View File

@ -437,10 +437,10 @@ func (i *InvoiceRegistry) checkHtlcParameters(invoice *channeldb.Invoice,
return channeldb.ErrInvoiceAlreadyCanceled return channeldb.ErrInvoiceAlreadyCanceled
} }
// If a payment has already been made, we only accept more payments if // If an invoice amount is specified, check that enough is paid. Also
// the amount is the exact same. This prevents probing with small // check this for duplicate payments if the invoice is already settled
// amounts on settled invoices to find out the receiver node. // or accepted.
if invoice.AmtPaid != 0 && amtPaid != invoice.AmtPaid { if invoice.Terms.Value > 0 && amtPaid < invoice.Terms.Value {
return ErrInvoiceAmountTooLow return ErrInvoiceAmountTooLow
} }
@ -468,11 +468,6 @@ func (i *InvoiceRegistry) checkHtlcParameters(invoice *channeldb.Invoice,
return ErrInvoiceExpiryTooSoon return ErrInvoiceExpiryTooSoon
} }
// If an invoice amount is specified, check that enough is paid.
if invoice.Terms.Value > 0 && amtPaid < invoice.Terms.Value {
return ErrInvoiceAmountTooLow
}
return nil return nil
} }

14
invoices/invoiceregistry_test.go
View File

@ -165,9 +165,9 @@ func TestSettleInvoice(t *testing.T) {
t.Fatal("expected settle event") t.Fatal("expected settle event")
} }
// Try to settle again with a higher amount. This should result in a // Try to settle again with a higher amount. This payment should also be
// cancel event because after a restart the amount should still be the // accepted, to prevent any change in behaviour for a paid invoice that
// same. New HTLCs with a different amount should be rejected. // may open up a probe vector.
event, err = registry.NotifyExitHopHtlc( event, err = registry.NotifyExitHopHtlc(
hash, amtPaid+600, testHtlcExpiry, testCurrentHeight, hash, amtPaid+600, testHtlcExpiry, testCurrentHeight,
hodlChan, nil, hodlChan, nil,
@ -175,12 +175,12 @@ func TestSettleInvoice(t *testing.T) {
if err != nil { if err != nil {
t.Fatalf("unexpected NotifyExitHopHtlc error: %v", err) t.Fatalf("unexpected NotifyExitHopHtlc error: %v", err)
} }
if event.Preimage != nil { if event.Preimage == nil {
t.Fatal("expected cancel event") t.Fatal("expected settle event")
} }
// Try to settle again with a lower amount. This should show the same // Try to settle again with a lower amount. This should fail just as it
// behaviour as settling with a higher amount. // would have failed if it were the first payment.
event, err = registry.NotifyExitHopHtlc( event, err = registry.NotifyExitHopHtlc(
hash, amtPaid-600, testHtlcExpiry, testCurrentHeight, hash, amtPaid-600, testHtlcExpiry, testCurrentHeight,
hodlChan, nil, hodlChan, nil,