From d7f81156d342e5106ef1fcf109b576d4c071ac88 Mon Sep 17 00:00:00 2001 From: Dylan Bourque Date: Fri, 9 Dec 2022 10:00:33 -0600 Subject: [PATCH] fix: avoid panic when decoding an invalid length-delimited field (#90) adds a bounds check to `Decoder.DecodeBytes()` to return `io.ErrUnexpectedEOF` instead of panicking if the encoded data is shorter than the encoded length --- decoder.go | 3 +++ decoder_test.go | 27 ++++++++++++++++++++------- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/decoder.go b/decoder.go index 7f359b6..06f7dae 100644 --- a/decoder.go +++ b/decoder.go @@ -140,6 +140,9 @@ func (d *Decoder) DecodeBytes() ([]byte, error) { if n == 0 { return nil, ErrInvalidVarintData } + if d.offset+n+int(l) > len(d.p) { + return nil, io.ErrUnexpectedEOF + } b := d.p[d.offset+n : d.offset+n+int(l)] d.offset += n + int(l) return b, nil diff --git a/decoder_test.go b/decoder_test.go index f8030cf..93e0a73 100644 --- a/decoder_test.go +++ b/decoder_test.go @@ -88,11 +88,12 @@ func TestDecodeString(t *testing.T) { func TestDecodeBytes(t *testing.T) { cases := []struct { - name string - fieldNum int - v []byte - wt csproto.WireType - expected []byte + name string + fieldNum int + v []byte + wt csproto.WireType + expected []byte + expectedErr error }{ { name: "empty slice", @@ -108,6 +109,14 @@ func TestDecodeBytes(t *testing.T) { wt: csproto.WireTypeLengthDelimited, expected: []byte{0x42, 0x11, 0x38}, }, + { + name: "invalid data", + fieldNum: 2, + v: []byte{0x12, 0x3, 0x42, 0x11}, // field data is truncated + wt: csproto.WireTypeLengthDelimited, + expected: []byte{0x42, 0x11, 0x38}, + expectedErr: io.ErrUnexpectedEOF, + }, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { @@ -118,8 +127,12 @@ func TestDecodeBytes(t *testing.T) { assert.NoError(t, err, "should not fail") got, err := dec.DecodeBytes() - assert.NoError(t, err) - assert.Equal(t, tc.expected, got) + if tc.expectedErr == nil { + assert.NoError(t, err) + assert.Equal(t, tc.expected, got) + } else { + assert.ErrorIs(t, err, tc.expectedErr) + } }) } }