Skip to content

Commit

Permalink
Merge pull request #2135 from SulaimanAminuBarkindo/improved-der-sign…
Browse files Browse the repository at this point in the history
…ature-validation

Add check for maximum signature length in ecdsa.ParseDERSignature
  • Loading branch information
Roasbeef authored Mar 18, 2024
2 parents 2fc99e0 + 9d2184e commit a90d3d9
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 5 deletions.
26 changes: 21 additions & 5 deletions btcec/ecdsa/signature.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,20 @@ var (
oneInitializer = []byte{0x01}
)

// MinSigLen is the minimum length of a DER encoded signature and is when both R
// and S are 1 byte each.
// 0x30 + <1-byte> + 0x02 + 0x01 + <byte> + 0x2 + 0x01 + <byte>
const MinSigLen = 8
const (
// MinSigLen is the minimum length of a DER encoded signature and is when both R
// and S are 1 byte each.
// 0x30 + <1-byte> + 0x02 + 0x01 + <byte> + 0x2 + 0x01 + <byte>
MinSigLen = 8

// MaxSigLen is the maximum length of a DER encoded signature and is
// when both R and S are 33 bytes each. It is 33 bytes because a
// 256-bit integer requires 32 bytes and an additional leading null byte
// might be required if the high bit is set in the value.
//
// 0x30 + <1-byte> + 0x02 + 0x21 + <33 bytes> + 0x2 + 0x21 + <33 bytes>
MaxSigLen = 72
)

// canonicalPadding checks whether a big-endian encoded integer could
// possibly be misinterpreted as a negative number (even though OpenSSL
Expand Down Expand Up @@ -68,9 +78,15 @@ func parseSig(sigStr []byte, der bool) (*Signature, error) {
// 0x30 <length of whole message> <0x02> <length of R> <R> 0x2
// <length of S> <S>.

if len(sigStr) < MinSigLen {
// The signature must adhere to the minimum and maximum allowed length.
totalSigLen := len(sigStr)
if totalSigLen < MinSigLen {
return nil, errors.New("malformed signature: too short")
}
if der && totalSigLen > MaxSigLen {
return nil, errors.New("malformed signature: too long")
}

// 0x30
index := 0
if sigStr[index] != 0x30 {
Expand Down
15 changes: 15 additions & 0 deletions btcec/ecdsa/signature_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,21 @@ var signatureTests = []signatureTest{
der: false,
isValid: false,
},
{
name: "Long signature.",
sig: []byte{0x30, 0x44, 0x02, 0x20, 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, 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, 0x91,
0x17, 0x90, 0xda, 0x42, 0xca, 0xaf, 0x19, 0x7d, 0xb4,
},
der: true,
isValid: false,
},
}

func TestSignatures(t *testing.T) {
Expand Down

0 comments on commit a90d3d9

Please sign in to comment.