Commit 0c8fe346 authored by Rob Pike's avatar Rob Pike

encoding/gob: more cleanups handling slice length

Fix the other places the slice length was being believed, and refactor
the code to use a single function to unify the check.

Fixes #10273.

Change-Id: Ia62b25203fbe87c95d71a70ebc1db8d202eaa4a4
Reviewed-on: https://go-review.googlesource.com/8511Reviewed-by: 's avatarBrad Fitzpatrick <bradfitz@golang.org>
parent d3252a2d
...@@ -182,6 +182,17 @@ func (state *decoderState) decodeInt() int64 { ...@@ -182,6 +182,17 @@ func (state *decoderState) decodeInt() int64 {
return int64(x >> 1) return int64(x >> 1)
} }
// getLength decodes the next uint and makes sure it is a possible
// size for a data item that follows, which means it must fit in a
// non-negative int and fit in the buffer.
func (state *decoderState) getLength() (int, bool) {
n := int(state.decodeUint())
if n < 0 || state.b.Len() < n || tooBig <= n {
return 0, false
}
return n, true
}
// decOp is the signature of a decoding operator for a given type. // decOp is the signature of a decoding operator for a given type.
type decOp func(i *decInstr, state *decoderState, v reflect.Value) type decOp func(i *decInstr, state *decoderState, v reflect.Value)
...@@ -363,16 +374,9 @@ func decComplex128(i *decInstr, state *decoderState, value reflect.Value) { ...@@ -363,16 +374,9 @@ func decComplex128(i *decInstr, state *decoderState, value reflect.Value) {
// describing the data. // describing the data.
// uint8 slices are encoded as an unsigned count followed by the raw bytes. // uint8 slices are encoded as an unsigned count followed by the raw bytes.
func decUint8Slice(i *decInstr, state *decoderState, value reflect.Value) { func decUint8Slice(i *decInstr, state *decoderState, value reflect.Value) {
u := state.decodeUint() n, ok := state.getLength()
n := int(u) if !ok {
if n < 0 || uint64(n) != u { errorf("bad %s slice length: %d", value.Type(), n)
errorf("length of %s exceeds input size (%d bytes)", value.Type(), u)
}
if n > state.b.Len() {
errorf("%s data too long for buffer: %d", value.Type(), n)
}
if n > tooBig {
errorf("byte slice too big: %d", n)
} }
if value.Cap() < n { if value.Cap() < n {
value.Set(reflect.MakeSlice(value.Type(), n, n)) value.Set(reflect.MakeSlice(value.Type(), n, n))
...@@ -388,13 +392,9 @@ func decUint8Slice(i *decInstr, state *decoderState, value reflect.Value) { ...@@ -388,13 +392,9 @@ func decUint8Slice(i *decInstr, state *decoderState, value reflect.Value) {
// describing the data. // describing the data.
// Strings are encoded as an unsigned count followed by the raw bytes. // Strings are encoded as an unsigned count followed by the raw bytes.
func decString(i *decInstr, state *decoderState, value reflect.Value) { func decString(i *decInstr, state *decoderState, value reflect.Value) {
u := state.decodeUint() n, ok := state.getLength()
n := int(u) if !ok {
if n < 0 || uint64(n) != u || n > state.b.Len() { errorf("bad %s slice length: %d", value.Type(), n)
errorf("length of %s exceeds input size (%d bytes)", value.Type(), u)
}
if n > state.b.Len() {
errorf("%s data too long for buffer: %d", value.Type(), n)
} }
// Read the data. // Read the data.
data := make([]byte, n) data := make([]byte, n)
...@@ -406,7 +406,11 @@ func decString(i *decInstr, state *decoderState, value reflect.Value) { ...@@ -406,7 +406,11 @@ func decString(i *decInstr, state *decoderState, value reflect.Value) {
// ignoreUint8Array skips over the data for a byte slice value with no destination. // ignoreUint8Array skips over the data for a byte slice value with no destination.
func ignoreUint8Array(i *decInstr, state *decoderState, value reflect.Value) { func ignoreUint8Array(i *decInstr, state *decoderState, value reflect.Value) {
b := make([]byte, state.decodeUint()) n, ok := state.getLength()
if !ok {
errorf("slice length too large")
}
b := make([]byte, n)
state.b.Read(b) state.b.Read(b)
} }
...@@ -688,8 +692,8 @@ func (dec *Decoder) ignoreInterface(state *decoderState) { ...@@ -688,8 +692,8 @@ func (dec *Decoder) ignoreInterface(state *decoderState) {
error_(dec.err) error_(dec.err)
} }
// At this point, the decoder buffer contains a delimited value. Just toss it. // At this point, the decoder buffer contains a delimited value. Just toss it.
n := int(state.decodeUint()) n, ok := state.getLength()
if n < 0 || state.b.Len() < n { if !ok {
errorf("bad interface encoding: length too large for buffer") errorf("bad interface encoding: length too large for buffer")
} }
state.b.Drop(n) state.b.Drop(n)
......
...@@ -968,3 +968,17 @@ func TestErrorBadDrop(t *testing.T) { ...@@ -968,3 +968,17 @@ func TestErrorBadDrop(t *testing.T) {
t.Fatalf("decode: expected interface encoding error, got %s", err.Error()) t.Fatalf("decode: expected interface encoding error, got %s", err.Error())
} }
} }
// Don't crash, just give error with corrupted slice.
// Issue 10273.
func TestErrorBadSliceLength(t *testing.T) {
data := []byte{0x13, 0x0a, 0x00, 0xfb, 0x5d, 0xad, 0x0b, 0xf8, 0xff, 0x02, 0x02, 0x63, 0xe7, 0x00, 0x02, 0xfa, 0x28, 0x02, 0x02, 0x02, 0xa8, 0x98, 0x59}
d := NewDecoder(bytes.NewReader(data))
err := d.Decode(nil)
if err == nil {
t.Fatal("decode: no error")
}
if !strings.Contains(err.Error(), "slice length too large") {
t.Fatalf("decode: expected slice length too large error, got %s", err.Error())
}
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment