encoding/binary: read at most MaxVarintLen64 bytes in ReadUvarint

This CL ensures that ReadUvarint consumes only a limited
amount of input (instead of an unbounded amount).

On some inputs, ReadUvarint could read an arbitrary number
of bytes before deciding to return an overflow error.
After this CL, ReadUvarint returns that same overflow
error sooner, after reading at most MaxVarintLen64 bytes.

Fix authored by Robert Griesemer and Filippo Valsorda.

Thanks to Diederik Loerakker, Jonny Rhea, Raúl Kripalani,
and Preston Van Loon for reporting this.

Fixes #40618
Fixes CVE-2020-16845

Change-Id: Ie0cb15972f14c38b7cf7af84c45c4ce54909bb8f
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/812099
Reviewed-by: Filippo Valsorda <valsorda@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/247120
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alexander Rakoczy <alex@golang.org>
This commit is contained in:
Katie Hockman
2020-08-04 11:45:32 -04:00
parent 6f08e89ec3
commit 027d7241ce
2 changed files with 15 additions and 8 deletions

View File

@@ -106,13 +106,13 @@ var overflow = errors.New("binary: varint overflows a 64-bit integer")
func ReadUvarint(r io.ByteReader) (uint64, error) { func ReadUvarint(r io.ByteReader) (uint64, error) {
var x uint64 var x uint64
var s uint var s uint
for i := 0; ; i++ { for i := 0; i < MaxVarintLen64; i++ {
b, err := r.ReadByte() b, err := r.ReadByte()
if err != nil { if err != nil {
return x, err return x, err
} }
if b < 0x80 { if b < 0x80 {
if i > 9 || i == 9 && b > 1 { if i == 9 && b > 1 {
return x, overflow return x, overflow
} }
return x | uint64(b)<<s, nil return x | uint64(b)<<s, nil
@@ -120,6 +120,7 @@ func ReadUvarint(r io.ByteReader) (uint64, error) {
x |= uint64(b&0x7f) << s x |= uint64(b&0x7f) << s
s += 7 s += 7
} }
return x, overflow
} }
// ReadVarint reads an encoded signed integer from r and returns it as an int64. // ReadVarint reads an encoded signed integer from r and returns it as an int64.

View File

@@ -121,21 +121,27 @@ func TestBufferTooSmall(t *testing.T) {
} }
} }
func testOverflow(t *testing.T, buf []byte, n0 int, err0 error) { func testOverflow(t *testing.T, buf []byte, x0 uint64, n0 int, err0 error) {
x, n := Uvarint(buf) x, n := Uvarint(buf)
if x != 0 || n != n0 { if x != 0 || n != n0 {
t.Errorf("Uvarint(%v): got x = %d, n = %d; want 0, %d", buf, x, n, n0) t.Errorf("Uvarint(%v): got x = %d, n = %d; want 0, %d", buf, x, n, n0)
} }
x, err := ReadUvarint(bytes.NewReader(buf)) r := bytes.NewReader(buf)
if x != 0 || err != err0 { len := r.Len()
t.Errorf("ReadUvarint(%v): got x = %d, err = %s; want 0, %s", buf, x, err, err0) x, err := ReadUvarint(r)
if x != x0 || err != err0 {
t.Errorf("ReadUvarint(%v): got x = %d, err = %s; want %d, %s", buf, x, err, x0, err0)
}
if read := len - r.Len(); read > MaxVarintLen64 {
t.Errorf("ReadUvarint(%v): read more than MaxVarintLen64 bytes, got %d", buf, read)
} }
} }
func TestOverflow(t *testing.T) { func TestOverflow(t *testing.T) {
testOverflow(t, []byte{0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x2}, -10, overflow) testOverflow(t, []byte{0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x2}, 0, -10, overflow)
testOverflow(t, []byte{0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x1, 0, 0}, -13, overflow) testOverflow(t, []byte{0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x1, 0, 0}, 0, -13, overflow)
testOverflow(t, []byte{0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF}, 1<<64-1, 0, overflow) // 11 bytes, should overflow
} }
func TestNonCanonicalZero(t *testing.T) { func TestNonCanonicalZero(t *testing.T) {