lnwire: add length validation in NewSigFromRawSignature

lnwire/signature.go

@@ -1,6 +1,7 @@
 package lnwire
 
 import (
+	"errors"
 	"fmt"
 
 	"github.com/btcsuite/btcd/btcec/v2/ecdsa"
@@ -13,23 +14,61 @@ import (
 // signature (raw bytes and *ecdsa.Signature).
 type Sig [64]byte
 
+var (
+	errSigTooShort = errors.New("malformed signature: too short")
+	errBadLength   = errors.New("malformed signature: bad length")
+	errBadRLength  = errors.New("malformed signature: bogus R length")
+	errBadSLength  = errors.New("malformed signature: bogus S length")
+	errRTooLong    = errors.New("R is over 32 bytes long without padding")
+	errSTooLong    = errors.New("S is over 32 bytes long without padding")
+)
+
 // NewSigFromRawSignature returns a Sig from a Bitcoin raw signature encoded in
 // the canonical DER encoding.
 func NewSigFromRawSignature(sig []byte) (Sig, error) {
 	var b Sig
 
-	if len(sig) == 0 {
+	// Check the total length is above the minimal.
-		return b, fmt.Errorf("cannot decode empty signature")
+	if len(sig) < ecdsa.MinSigLen {
+		return b, errSigTooShort
 	}
 
-	// Extract lengths of R and S. The DER representation is laid out as
+	// The DER representation is laid out as:
-	// 0x30 <length> 0x02 <length r> r 0x02 <length s> s
+	//   0x30 <length> 0x02 <length r> r 0x02 <length s> s
-	// which means the length of R is the 4th byte and the length of S
+	// which means the length of R is the 4th byte and the length of S is
-	// is the second byte after R ends. 0x02 signifies a length-prefixed,
+	// the second byte after R ends. 0x02 signifies a length-prefixed,
 	// zero-padded, big-endian bigint. 0x30 signifies a DER signature.
 	// See the Serialize() method for ecdsa.Signature for details.
-	rLen := sig[3]
+
-	sLen := sig[5+rLen]
+	// Reading <length>, remaining: [0x02 <length r> r 0x02 <length s> s]
+	sigLen := int(sig[1])
+
+	// siglen should be less than the entire message and greater than
+	// the minimal message size.
+	if sigLen+2 > len(sig) || sigLen+2 < ecdsa.MinSigLen {
+		return b, errBadLength
+	}
+
+	// Reading <length r>, remaining: [r 0x02 <length s> s]
+	rLen := int(sig[3])
+
+	// rLen must be positive and must be able to fit in other elements.
+	// Assuming s is one byte, then we have 0x30, <length>, 0x20,
+	// <length r>, 0x20, <length s>, s, a total of 7 bytes.
+	if rLen <= 0 || rLen+7 > len(sig) {
+		return b, errBadRLength
+	}
+
+	// Reading <length s>, remaining: [s]
+	sLen := int(sig[5+rLen])
+
+	// S should be the rest of the string.
+	// sLen must be positive and must be able to fit in other elements.
+	// We know r is rLen bytes, and we have 0x30, <length>, 0x20,
+	// <length r>, 0x20, <length s>, a total of rLen+6 bytes.
+	if sLen <= 0 || sLen+rLen+6 > len(sig) {
+		return b, errBadSLength
+	}
 
 	// Check to make sure R and S can both fit into their intended buffers.
 	// We check S first because these code blocks decrement sLen and rLen
@@ -39,8 +78,7 @@ func NewSigFromRawSignature(sig []byte) (Sig, error) {
 	// check S first.
 	if sLen > 32 {
 		if (sLen > 33) || (sig[6+rLen] != 0x00) {
-			return b, fmt.Errorf("S is over 32 bytes long " +
+			return b, errSTooLong
-				"without padding")
 		}
 		sLen--
 		copy(b[64-sLen:], sig[7+rLen:])
@@ -51,8 +89,7 @@ func NewSigFromRawSignature(sig []byte) (Sig, error) {
 	// Do the same for R as we did for S
 	if rLen > 32 {
 		if (rLen > 33) || (sig[4] != 0x00) {
-			return b, fmt.Errorf("R is over 32 bytes long " +
+			return b, errRTooLong
-				"without padding")
 		}
 		rLen--
 		copy(b[32-rLen:], sig[5:5+rLen])

lnwire/signature_test.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/btcsuite/btcd/btcec/v2"
 	"github.com/btcsuite/btcd/btcec/v2/ecdsa"
+	"github.com/stretchr/testify/require"
 )
 
 func TestSignatureSerializeDeserialize(t *testing.T) {
@@ -92,3 +93,182 @@ func TestSignatureSerializeDeserialize(t *testing.T) {
 	// the signature from ModNScalar values which don't allow setting a
 	// value larger than N (hence the name mod n).
 }
+
+var (
+	// signatures from bitcoin blockchain tx
+	// 0437cd7f8525ceed2324359c2d0ba26006d92d85.
+	normalSig = []byte{
+		0x30, 0x44, 0x02, 0x20,
+		// r value
+		0x4e, 0x45, 0xe1, 0x69, 0x32, 0xb8, 0xaf, 0x51,
+		0x49, 0x61, 0xa1, 0xd3, 0xa1, 0xa2, 0x5f, 0xdf,
+		0x3f, 0x4f, 0x77, 0x32, 0xe9, 0xd6, 0x24, 0xc6,
+		0xc6, 0x15, 0x48, 0xab, 0x5f, 0xb8, 0xcd, 0x41,
+
+		0x02, 0x20,
+		// s value
+		0x18, 0x15, 0x22, 0xec, 0x8e, 0xca, 0x07, 0xde,
+		0x48, 0x60, 0xa4, 0xac, 0xdd, 0x12, 0x90, 0x9d,
+		0x83, 0x1c, 0xc5, 0x6c, 0xbb, 0xac, 0x46, 0x22,
+		0x08, 0x22, 0x21, 0xa8, 0x76, 0x8d, 0x1d, 0x09,
+	}
+
+	// minimal length with 1 byte r and 1 byte s.
+	minSig = []byte{
+		0x30, 0x06, 0x02, 0x01, 0x00, 0x02, 0x01, 0x00,
+	}
+
+	// sig length is below 6.
+	smallLenSig = []byte{
+		0x30, 0x05, 0x02, 0x01, 0x00, 0x02, 0x01, 0x00,
+	}
+
+	// sig length is above 6.
+	largeLenSig = []byte{
+		0x30, 0x07, 0x02, 0x01, 0x00, 0x02, 0x01, 0x00,
+	}
+
+	// r length is 2.
+	largeRSig = []byte{
+		0x30, 0x06, 0x02, 0x02, 0x00, 0x02, 0x01, 0x00,
+	}
+
+	// r length is 0.
+	smallRSig = []byte{
+		0x30, 0x06, 0x02, 0x00, 0x00, 0x02, 0x01, 0x00,
+	}
+
+	// s length is 2.
+	largeSSig = []byte{
+		0x30, 0x06, 0x02, 0x01, 0x00, 0x02, 0x02, 0x00,
+	}
+
+	// s length is 0.
+	smallSSig = []byte{
+		0x30, 0x06, 0x02, 0x01, 0x00, 0x02, 0x00, 0x00,
+	}
+
+	// r length is 33.
+	missPaddingRSig = []byte{
+		0x30, 0x25, 0x02, 0x21,
+		// r value with a wrong padding.
+		0xff,
+		0x4e, 0x45, 0xe1, 0x69, 0x32, 0xb8, 0xaf, 0x51,
+		0x49, 0x61, 0xa1, 0xd3, 0xa1, 0xa2, 0x5f, 0xdf,
+		0x3f, 0x4f, 0x77, 0x32, 0xe9, 0xd6, 0x24, 0xc6,
+		0xc6, 0x15, 0x48, 0xab, 0x5f, 0xb8, 0xcd, 0x41,
+		// s value is 0.
+		0x02, 0x01, 0x00,
+	}
+
+	// s length is 33.
+	missPaddingSSig = []byte{
+		// r value is 0.
+		0x30, 0x25, 0x02, 0x01, 0x00,
+		0x02, 0x21,
+		// s value with a wrong padding.
+		0xff,
+		0x18, 0x15, 0x22, 0xec, 0x8e, 0xca, 0x07, 0xde,
+		0x48, 0x60, 0xa4, 0xac, 0xdd, 0x12, 0x90, 0x9d,
+		0x83, 0x1c, 0xc5, 0x6c, 0xbb, 0xac, 0x46, 0x22,
+		0x08, 0x22, 0x21, 0xa8, 0x76, 0x8d, 0x1d, 0x09,
+	}
+)
+
+func TestNewSigFromRawSignature(t *testing.T) {
+	t.Parallel()
+	testCases := []struct {
+		name        string
+		rawSig      []byte
+		expectedErr error
+		expectedSig Sig
+	}{
+		{
+			name:        "valid signature",
+			rawSig:      normalSig,
+			expectedErr: nil,
+			expectedSig: Sig{
+				// r value
+				0x4e, 0x45, 0xe1, 0x69, 0x32, 0xb8, 0xaf, 0x51,
+				0x49, 0x61, 0xa1, 0xd3, 0xa1, 0xa2, 0x5f, 0xdf,
+				0x3f, 0x4f, 0x77, 0x32, 0xe9, 0xd6, 0x24, 0xc6,
+				0xc6, 0x15, 0x48, 0xab, 0x5f, 0xb8, 0xcd, 0x41,
+				// s value
+				0x18, 0x15, 0x22, 0xec, 0x8e, 0xca, 0x07, 0xde,
+				0x48, 0x60, 0xa4, 0xac, 0xdd, 0x12, 0x90, 0x9d,
+				0x83, 0x1c, 0xc5, 0x6c, 0xbb, 0xac, 0x46, 0x22,
+				0x08, 0x22, 0x21, 0xa8, 0x76, 0x8d, 0x1d, 0x09,
+			},
+		},
+		{
+			name:        "minimal length signature",
+			rawSig:      minSig,
+			expectedErr: nil,
+			// NOTE: r and s are both 0x00 here.
+			expectedSig: Sig{},
+		},
+		{
+			name:        "signature length too short",
+			rawSig:      []byte{0x30},
+			expectedErr: errSigTooShort,
+			expectedSig: Sig{},
+		},
+		{
+			name:        "sig length too large",
+			rawSig:      largeLenSig,
+			expectedErr: errBadLength,
+			expectedSig: Sig{},
+		},
+		{
+			name:        "sig length too small",
+			rawSig:      smallLenSig,
+			expectedErr: errBadLength,
+			expectedSig: Sig{},
+		},
+		{
+			name:        "r length too large",
+			rawSig:      largeRSig,
+			expectedErr: errBadRLength,
+			expectedSig: Sig{},
+		},
+		{
+			name:        "r length too small",
+			rawSig:      smallRSig,
+			expectedErr: errBadRLength,
+			expectedSig: Sig{},
+		},
+		{
+			name:        "s length too large",
+			rawSig:      largeSSig,
+			expectedErr: errBadSLength,
+			expectedSig: Sig{},
+		},
+		{
+			name:        "s length too small",
+			rawSig:      smallSSig,
+			expectedErr: errBadSLength,
+			expectedSig: Sig{},
+		},
+		{
+			name:        "missing padding in r",
+			rawSig:      missPaddingRSig,
+			expectedErr: errRTooLong,
+			expectedSig: Sig{},
+		},
+		{
+			name:        "missing padding in s",
+			rawSig:      missPaddingSSig,
+			expectedErr: errSTooLong,
+			expectedSig: Sig{},
+		},
+	}
+
+	for _, tc := range testCases {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			result, err := NewSigFromRawSignature(tc.rawSig)
+			require.Equal(t, tc.expectedErr, err)
+			require.Equal(t, tc.expectedSig, result)
+		})
+	}
+}