zpay32: fix decoding when payment request is too short

This commit fixes a panic that can result when a zpay32 payment request
that is too short (and possibly invalid) is attempted to be decoded.

To fix this bug, we now simply ensure that that after we decode the
zbase32 encoding, the resulting set of bytes is _exactly_ the length we
expect. A new error has been introduced to handle this case, and a
simple test has been added which ensures proper handling of short
payment requests.

Fixes #127.
This commit is contained in:
Alex Akselrod 2017-02-10 17:22:09 -07:00 committed by Olaoluwa Osuntokun
parent a2403d9c07
commit 55e693aa09
2 changed files with 21 additions and 1 deletions

@ -23,6 +23,11 @@ const invoiceSize = 33 + 32 + 8
// an error somewhere in the bitstream. // an error somewhere in the bitstream.
var ErrCheckSumMismatch = errors.New("the checksum is incorrect") var ErrCheckSumMismatch = errors.New("the checksum is incorrect")
// ErrDataTooShort is returned by the Decode function if when
// decoding an encoded payment request, the number of bytes decoded
// is too few for a valid invoice indicating invalid input.
var ErrDataTooShort = errors.New("the decoded data is too short")
// PaymentRequest is a bare-bones invoice for a payment within the Lightning // PaymentRequest is a bare-bones invoice for a payment within the Lightning
// Network. With the details of the invoice, the sender has all the data // Network. With the details of the invoice, the sender has all the data
// necessary to send a payment to the recipient. // necessary to send a payment to the recipient.
@ -90,7 +95,13 @@ func Decode(payData string) (*PaymentRequest, error) {
return nil, err return nil, err
} }
// With the bytes decoded, we first verify the checksum to ensure the // Check if there are at least enough bytes to represent the invoice
// and the checksum.
if len(payReqBytes) < invoiceSize+crc32.Size {
return nil, ErrDataTooShort
}
// With the bytes decoded, we verify the checksum to ensure the
// payment request wasn't altered in its decoded form. // payment request wasn't altered in its decoded form.
invoiceBytes := payReqBytes[:invoiceSize] invoiceBytes := payReqBytes[:invoiceSize]
generatedSum := checkSum(invoiceBytes) generatedSum := checkSum(invoiceBytes)

@ -94,3 +94,12 @@ func TestChecksumMismatch(t *testing.T) {
t.Fatalf("decode should fail with checksum mismatch, instead: %v", err) t.Fatalf("decode should fail with checksum mismatch, instead: %v", err)
} }
} }
func TestDecodeTooShort(t *testing.T) {
// We start with a pre-encoded too-short string.
payReqString := "ycyr8brdji"
if _, err := Decode(payReqString); err != ErrDataTooShort {
t.Fatalf("decode should fail with data too short, instead: %v", err)
}
}