Merge pull request #6314 from yyforyongyu/fix-sig-len

lnwire: add length validation in NewSigFromRawSignature
This commit is contained in:
Oliver Gugger 2022-03-11 09:44:11 +01:00 committed by GitHub
commit 41b91bd5be
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 242 additions and 15 deletions

View File

@ -44,6 +44,11 @@
arbitrator relying on htlcswitch to be started
first](https://github.com/lightningnetwork/lnd/pull/6214).
* [Added signature length
validation](https://github.com/lightningnetwork/lnd/pull/6314) when calling
`NewSigFromRawSignature`.
## Misc
* [An example systemd service file](https://github.com/lightningnetwork/lnd/pull/6033)

View File

@ -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 {
return b, fmt.Errorf("cannot decode empty signature")
// Check the total length is above the minimal.
if len(sig) < ecdsa.MinSigLen {
return b, errSigTooShort
}
// Extract lengths of R and S. The DER representation is laid out as
// 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
// is the second byte after R ends. 0x02 signifies a length-prefixed,
// The DER representation is laid out as:
// 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 is
// 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 " +
"without padding")
return b, errSTooLong
}
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 " +
"without padding")
return b, errRTooLong
}
rLen--
copy(b[32-rLen:], sig[5:5+rLen])

View File

@ -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)
})
}
}

View File

@ -21,6 +21,11 @@ import (
var (
testKeyLoc = keychain.KeyLocator{Family: keychain.KeyFamilyNodeKey}
// testSigBytes specifies a testing signature with the minimal length.
testSigBytes = []byte{
0x30, 0x06, 0x02, 0x01, 0x00, 0x02, 0x01, 0x00,
}
)
// randOutpoint creates a random wire.Outpoint.
@ -105,13 +110,13 @@ func createEdgePolicies(t *testing.T, channel *channeldb.OpenChannel,
ChannelID: channel.ShortChanID().ToUint64(),
ChannelFlags: dir1,
LastUpdate: time.Now(),
SigBytes: make([]byte, 64),
SigBytes: testSigBytes,
},
&channeldb.ChannelEdgePolicy{
ChannelID: channel.ShortChanID().ToUint64(),
ChannelFlags: dir2,
LastUpdate: time.Now(),
SigBytes: make([]byte, 64),
SigBytes: testSigBytes,
}
}
@ -209,7 +214,7 @@ func (g *mockGraph) ApplyChannelUpdate(update *lnwire.ChannelUpdate) error {
ChannelID: update.ShortChannelID.ToUint64(),
ChannelFlags: update.ChannelFlags,
LastUpdate: timestamp,
SigBytes: make([]byte, 64),
SigBytes: testSigBytes,
}
if update1 {