@@ -414,6 +414,10 @@ func writeNewFormatLength(w *bytes.Buffer, length int) {
}
// parseASN1Signature extracts r and s from an ASN.1 DER encoded ECDSA signature.
+//
+// Each intermediate slice access is bounds-checked against len(der). A truncated
+// or malformed signature produces a typed error rather than an index-out-of-range
+// panic; the minimum-length check up front only rules out obvious runts (#613).
func parseASN1Signature(der []byte) (r, s []byte, err error) {
// ASN.1 SEQUENCE { INTEGER r, INTEGER s }
if len(der) < 6 || der[0] != 0x30 {
@@ -423,22 +427,34 @@ func parseASN1Signature(der []byte) (r, s []byte, err error) {
pos := 2 // skip SEQUENCE tag and length
// Parse R
- if der[pos] != 0x02 {
+ if pos >= len(der) || der[pos] != 0x02 {
return nil, nil, fmt.Errorf("expected INTEGER tag for R")
}
pos++
+ if pos >= len(der) {
+ return nil, nil, fmt.Errorf("ASN.1 signature truncated before R length")
+ }
rLen := int(der[pos])
pos++
+ if pos+rLen > len(der) {
+ return nil, nil, fmt.Errorf("ASN.1 signature truncated: R length overflow")
+ }
rVal := new(big.Int).SetBytes(der[pos : pos+rLen])
pos += rLen
// Parse S
- if der[pos] != 0x02 {
+ if pos >= len(der) || der[pos] != 0x02 {
return nil, nil, fmt.Errorf("expected INTEGER tag for S")
}
pos++
+ if pos >= len(der) {
+ return nil, nil, fmt.Errorf("ASN.1 signature truncated before S length")
+ }
sLen := int(der[pos])
pos++
+ if pos+sLen > len(der) {
+ return nil, nil, fmt.Errorf("ASN.1 signature truncated: S length overflow")
+ }
sVal := new(big.Int).SetBytes(der[pos : pos+sLen])
return rVal.Bytes(), sVal.Bytes(), nil
@@ -0,0 +1,74 @@
+package pgp
+
+import (
+ "strings"
+ "testing"
+)
+
+// TestParseASN1Signature_TruncatedDoesNotPanic covers the bounds-check path
+// added for #613. Each input would have panicked in the original parser
+// with "index out of range"; here we expect a typed error instead.
+func TestParseASN1Signature_TruncatedDoesNotPanic(t *testing.T) {
+ cases := []struct {
+ name string
+ der []byte
+ wantErr string
+ }{
+ {
+ // Length byte declares 0x10 bytes of R but only 1 byte follows.
+ name: "R length overruns buffer",
+ der: []byte{0x30, 0x06, 0x02, 0x10, 0xAA, 0x00},
+ wantErr: "R length overflow",
+ },
+ {
+ // Length byte declares 0x10 bytes of S but only 1 byte follows.
+ name: "S length overruns buffer",
+ der: []byte{0x30, 0x06, 0x02, 0x01, 0x01, 0x02, 0x10, 0xAA},
+ wantErr: "S length overflow",
+ },
+ {
+ // Valid R, then no S block at all.
+ name: "missing S after R",
+ der: []byte{0x30, 0x06, 0x02, 0x01, 0x01, 0x00},
+ wantErr: "expected INTEGER tag for S",
+ },
+ }
+
+ for _, tc := range cases {
+ tc := tc
+ t.Run(tc.name, func(t *testing.T) {
+ // The test must not panic: the fix replaces panics with errors.
+ defer func() {
+ if r := recover(); r != nil {
+ t.Fatalf("parseASN1Signature panicked: %v", r)
+ }
+ }()
+ _, _, err := parseASN1Signature(tc.der)
+ if err == nil {
+ t.Fatalf("want error, got nil")
+ }
+ if !strings.Contains(err.Error(), tc.wantErr) {
+ t.Fatalf("error = %q, want it to mention %q", err.Error(), tc.wantErr)
+ }
+ })
+ }
+}
+
+// TestParseASN1Signature_WellFormed guards against regressions in the
+// happy path: a minimal SEQUENCE { INTEGER, INTEGER } must still decode
+// to the original r and s bytes.
+func TestParseASN1Signature_WellFormed(t *testing.T) {
+ // SEQUENCE (6 bytes) { INTEGER 0x01, INTEGER 0x02 }
+ der := []byte{0x30, 0x06, 0x02, 0x01, 0x01, 0x02, 0x01, 0x02}
+
+ r, s, err := parseASN1Signature(der)
+ if err != nil {
+ t.Fatalf("unexpected error: %v", err)
+ }
+ if len(r) != 1 || r[0] != 0x01 {
+ t.Errorf("r = %x, want 01", r)
+ }
+ if len(s) != 1 || s[0] != 0x02 {
+ t.Errorf("s = %x, want 02", s)
+ }
+}