encoding/asn1: Reject invalid INTEGERs.

The empty string is not a valid DER integer. DER also requires that values be
minimally-encoded, so excess padding with leading 0s (0xff for negative
numbers) is forbidden. (These rules also apply to BER, incidentally.)

Fixes #12622.

Change-Id: I041f94e34a8afa29dbf94dd8fc450944bc91c9c3
Reviewed-on: https://go-review.googlesource.com/17008
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

src/encoding/asn1/asn1.go

@@ -71,9 +71,28 @@ func parseBool(bytes []byte) (ret bool, err error) {
 
 // INTEGER
 
+// checkInteger returns nil if the given bytes are a valid DER-encoded
+// INTEGER and an error otherwise.
+func checkInteger(bytes []byte) error {
+	if len(bytes) == 0 {
+		return StructuralError{"empty integer"}
+	}
+	if len(bytes) == 1 {
+		return nil
+	}
+	if (bytes[0] == 0 && bytes[1]&0x80 == 0) || (bytes[0] == 0xff && bytes[1]&0x80 == 0x80) {
+		return StructuralError{"integer not minimally-encoded"}
+	}
+	return nil
+}
+
 // parseInt64 treats the given bytes as a big-endian, signed integer and
 // returns the result.
 func parseInt64(bytes []byte) (ret int64, err error) {
+	err = checkInteger(bytes)
+	if err != nil {
+		return
+	}
 	if len(bytes) > 8 {
 		// We'll overflow an int64 in this case.
 		err = StructuralError{"integer too large"}
@@ -93,6 +112,9 @@ func parseInt64(bytes []byte) (ret int64, err error) {
 // parseInt treats the given bytes as a big-endian, signed integer and returns
 // the result.
 func parseInt32(bytes []byte) (int32, error) {
+	if err := checkInteger(bytes); err != nil {
+		return 0, err
+	}
 	ret64, err := parseInt64(bytes)
 	if err != nil {
 		return 0, err
@@ -107,7 +129,10 @@ var bigOne = big.NewInt(1)
 
 // parseBigInt treats the given bytes as a big-endian, signed integer and returns
 // the result.
-func parseBigInt(bytes []byte) *big.Int {
+func parseBigInt(bytes []byte) (*big.Int, error) {
+	if err := checkInteger(bytes); err != nil {
+		return nil, err
+	}
 	ret := new(big.Int)
 	if len(bytes) > 0 && bytes[0]&0x80 == 0x80 {
 		// This is a negative number.
@@ -118,10 +143,10 @@ func parseBigInt(bytes []byte) *big.Int {
 		ret.SetBytes(notBytes)
 		ret.Add(ret, bigOne)
 		ret.Neg(ret)
-		return ret
+		return ret, nil
 	}
 	ret.SetBytes(bytes)
-	return ret
+	return ret, nil
 }
 
 // BIT STRING
@@ -777,8 +802,11 @@ func parseField(v reflect.Value, bytes []byte, initOffset int, params fieldParam
 		v.SetBool(true)
 		return
 	case bigIntType:
-		parsedInt := parseBigInt(innerBytes)
+		parsedInt, err1 := parseBigInt(innerBytes)
-		v.Set(reflect.ValueOf(parsedInt))
+		if err1 == nil {
+			v.Set(reflect.ValueOf(parsedInt))
+		}
+		err = err1
 		return
 	}
 	switch val := v; val.Kind() {

src/encoding/asn1/asn1_test.go

@@ -53,10 +53,12 @@ var int64TestData = []int64Test{
 	{[]byte{0x01, 0x00}, true, 256},
 	{[]byte{0x80}, true, -128},
 	{[]byte{0xff, 0x7f}, true, -129},
-	{[]byte{0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}, true, -1},
 	{[]byte{0xff}, true, -1},
 	{[]byte{0x80, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}, true, -9223372036854775808},
 	{[]byte{0x80, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}, false, 0},
+	{[]byte{}, false, 0},
+	{[]byte{0x00, 0x7f}, false, 0},
+	{[]byte{0xff, 0xf0}, false, 0},
 }
 
 func TestParseInt64(t *testing.T) {
@@ -84,10 +86,12 @@ var int32TestData = []int32Test{
 	{[]byte{0x01, 0x00}, true, 256},
 	{[]byte{0x80}, true, -128},
 	{[]byte{0xff, 0x7f}, true, -129},
-	{[]byte{0xff, 0xff, 0xff, 0xff}, true, -1},
 	{[]byte{0xff}, true, -1},
 	{[]byte{0x80, 0x00, 0x00, 0x00}, true, -2147483648},
 	{[]byte{0x80, 0x00, 0x00, 0x00, 0x00}, false, 0},
+	{[]byte{}, false, 0},
+	{[]byte{0x00, 0x7f}, false, 0},
+	{[]byte{0xff, 0xf0}, false, 0},
 }
 
 func TestParseInt32(t *testing.T) {
@@ -104,27 +108,36 @@ func TestParseInt32(t *testing.T) {
 
 var bigIntTests = []struct {
 	in     []byte
+	ok     bool
 	base10 string
 }{
-	{[]byte{0xff}, "-1"},
+	{[]byte{0xff}, true, "-1"},
-	{[]byte{0x00}, "0"},
+	{[]byte{0x00}, true, "0"},
-	{[]byte{0x01}, "1"},
+	{[]byte{0x01}, true, "1"},
-	{[]byte{0x00, 0xff}, "255"},
+	{[]byte{0x00, 0xff}, true, "255"},
-	{[]byte{0xff, 0x00}, "-256"},
+	{[]byte{0xff, 0x00}, true, "-256"},
-	{[]byte{0x01, 0x00}, "256"},
+	{[]byte{0x01, 0x00}, true, "256"},
+	{[]byte{}, false, ""},
+	{[]byte{0x00, 0x7f}, false, ""},
+	{[]byte{0xff, 0xf0}, false, ""},
 }
 
 func TestParseBigInt(t *testing.T) {
 	for i, test := range bigIntTests {
-		ret := parseBigInt(test.in)
+		ret, err := parseBigInt(test.in)
-		if ret.String() != test.base10 {
+		if (err == nil) != test.ok {
-			t.Errorf("#%d: bad result from %x, got %s want %s", i, test.in, ret.String(), test.base10)
+			t.Errorf("#%d: Incorrect error result (did fail? %v, expected: %v)", i, err == nil, test.ok)
 		}
-		fw := newForkableWriter()
+		if test.ok {
-		marshalBigInt(fw, ret)
+			if ret.String() != test.base10 {
-		result := fw.Bytes()
+				t.Errorf("#%d: bad result from %x, got %s want %s", i, test.in, ret.String(), test.base10)
-		if !bytes.Equal(result, test.in) {
+			}
-			t.Errorf("#%d: got %x from marshaling %s, want %x", i, result, ret, test.in)
+			fw := newForkableWriter()
+			marshalBigInt(fw, ret)
+			result := fw.Bytes()
+			if !bytes.Equal(result, test.in) {
+				t.Errorf("#%d: got %x from marshaling %s, want %x", i, result, ret, test.in)
+			}
 		}
 	}
 }