From 7f96e266ec684943acfc1164a18d2cf005e03ef6 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Mon, 4 Jan 2016 16:16:28 -0800 Subject: [PATCH] encoding/asn1: fix off-by-one in parseBase128Int. parseBase128Int compares |shifted| with four, seemingly to ensure the result fits in an int32 on 32-bit platforms where int is 32-bit. However, there is an off-by-one in this logic, so it actually allows five shifts, making the maximum tag number or OID component 2^35-1. Fix this so the maximum is 2^28-1 which should be plenty for OID components and tag numbers while not overflowing on 32-bit platforms. Change-Id: If825b30cc53a0fc08e68ea1a24d265e7eb1a13a4 Reviewed-on: https://go-review.googlesource.com/18225 Reviewed-by: Adam Langley Reviewed-by: Brad Fitzpatrick Reviewed-by: Russ Cox --- src/encoding/asn1/asn1.go | 2 +- src/encoding/asn1/asn1_test.go | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/encoding/asn1/asn1.go b/src/encoding/asn1/asn1.go index 0070ea82a7..8bafefd52b 100644 --- a/src/encoding/asn1/asn1.go +++ b/src/encoding/asn1/asn1.go @@ -294,7 +294,7 @@ type Flag bool func parseBase128Int(bytes []byte, initOffset int) (ret, offset int, err error) { offset = initOffset for shifted := 0; offset < len(bytes); shifted++ { - if shifted > 4 { + if shifted == 4 { err = StructuralError{"base 128 integer too large"} return } diff --git a/src/encoding/asn1/asn1_test.go b/src/encoding/asn1/asn1_test.go index 509a2cb25e..e0e833123b 100644 --- a/src/encoding/asn1/asn1_test.go +++ b/src/encoding/asn1/asn1_test.go @@ -380,6 +380,8 @@ var tagAndLengthData = []tagAndLengthTest{ {[]byte{0xa0, 0x84, 0x80, 0x00, 0x00, 0x00}, false, tagAndLength{}}, // Long length form may not be used for lengths that fit in short form. {[]byte{0xa0, 0x81, 0x7f}, false, tagAndLength{}}, + // Tag numbers which would overflow int32 are rejected. (The value below is 2^31.) + {[]byte{0x1f, 0x88, 0x80, 0x80, 0x80, 0x00, 0x00}, false, tagAndLength{}}, } func TestParseTagAndLength(t *testing.T) { -- 2.50.0