zpay32: check invoice length while decoding

This commit checks that the size of the bech32 encoded invoice is not
greater than 7092 bytes, which is the maximum number of bytes that can
fit into a QR code. This mitigates a potential DoS vector where an attacker
could craft a very large bech32 invoice string containing an absurd amount
of route and/or hop hints. If sent to an application that processes
payment requests, this would allocate a burdensome amount of memory
due to the public key parsing for each route/hop hint.

For a 1.7MB payment request, this yielded about 38MB in allocations
from just parsing public keys:

```
   45.51MB  7.31% 92.07%    45.51MB  7.31%  math/big.nat.make
   25.50MB  4.09% 96.16%    25.50MB  4.09%  github.com/lightningnetwork/lnd/zpay32.bech32VerifyChecksum
       1MB  0.16% 96.32%    39.50MB  6.34%  github.com/lightningnetwork/lnd/zpay32.parseRouteHint
       1MB  0.16% 96.48%    33.50MB  5.38%  github.com/btcsuite/btcd/btcec.decompressPoint
    0.50MB  0.08% 96.56%     7.50MB  1.20%  crypto/elliptic.(*CurveParams).doubleJacobian
    0.50MB  0.08% 96.64%       38MB  6.10%  github.com/btcsuite/btcd/btcec.ParsePubKey
         0     0% 96.64%       12MB  1.93%  crypto/ecdsa.Verify
         0     0% 96.64%        8MB  1.28%  crypto/elliptic.(*CurveParams).ScalarBaseMult
         0     0% 96.64%       12MB  1.93%  crypto/elliptic.(*CurveParams).ScalarMult
```

With this change, memory usage will be far lower as decoding will exit
early with an error if the invoice is too large.
This commit is contained in:
nsa 2019-09-13 23:17:56 -04:00
parent 20a5ee2f1e
commit 0f6e11c35f
No known key found for this signature in database
GPG Key ID: 118759E83439A9B1
2 changed files with 54 additions and 15 deletions

@ -3,6 +3,7 @@ package zpay32
import (
"bytes"
"encoding/binary"
"errors"
"fmt"
"strings"
"time"
@ -71,12 +72,20 @@ const (
// fieldType9 contains one or more bytes for signaling features
// supported or required by the receiver.
fieldType9 = 5
// maxInvoiceLength is the maximum total length an invoice can have.
// This is chosen to be the maximum number of bytes that can fit into a
// single QR code: https://en.wikipedia.org/wiki/QR_code#Storage
maxInvoiceLength = 7089
)
var (
// InvoiceFeatures holds the set of all known feature bits that are
// exposed as BOLT 11 features.
InvoiceFeatures = map[lnwire.FeatureBit]string{}
// ErrInvoiceTooLarge is returned when an invoice exceeds maxInvoiceLength.
ErrInvoiceTooLarge = errors.New("invoice is too large")
)
// MessageSigner is passed to the Encode method to provide a signature
@ -263,6 +272,12 @@ func NewInvoice(net *chaincfg.Params, paymentHash [32]byte,
func Decode(invoice string, net *chaincfg.Params) (*Invoice, error) {
decodedInvoice := Invoice{}
// Before bech32 decoding the invoice, make sure that it is not too large.
// This is done as an anti-DoS measure since bech32 decoding is expensive.
if len(invoice) > maxInvoiceLength {
return nil, ErrInvoiceTooLarge
}
// Decode the invoice using the modified bech32 decoder.
hrp, data, err := decodeBech32(invoice)
if err != nil {
@ -467,6 +482,12 @@ func (invoice *Invoice) Encode(signer MessageSigner) (string, error) {
return "", err
}
// Before returning, check that the bech32 encoded string is not greater
// than our largest supported invoice size.
if len(b32) > maxInvoiceLength {
return "", ErrInvoiceTooLarge
}
return b32, nil
}
@ -518,21 +539,6 @@ func validateInvoice(invoice *Invoice) error {
return fmt.Errorf("neither description nor description hash set")
}
// We'll restrict invoices to include up to 20 different private route
// hints. We do this to avoid overly large invoices.
if len(invoice.RouteHints) > 20 {
return fmt.Errorf("too many private routes: %d",
len(invoice.RouteHints))
}
// Each route hint can have at most 20 hops.
for i, routeHint := range invoice.RouteHints {
if len(routeHint) > 20 {
return fmt.Errorf("route hint %d has too many extra "+
"hops: %d", i, len(routeHint))
}
}
// Check that we support the field lengths.
if len(invoice.PaymentHash) != 32 {
return fmt.Errorf("unsupported payment hash length: %d",
@ -870,6 +876,7 @@ func parseRouteHint(data []byte) ([]HopHint, error) {
return nil, err
}
// Check that base256Data is a multiple of hopHintLen.
if len(base256Data)%hopHintLen != 0 {
return nil, fmt.Errorf("expected length multiple of %d bytes, "+
"got %d", hopHintLen, len(base256Data))

File diff suppressed because one or more lines are too long