Skip to content

Commit

Permalink
fix: avoid panic when decoding an invalid length-delimited field (#90)
Browse files Browse the repository at this point in the history
adds a bounds check to `Decoder.DecodeBytes()` to return `io.ErrUnexpectedEOF` instead of panicking
if the encoded data is shorter than the encoded length
  • Loading branch information
dylan-bourque authored Dec 9, 2022
1 parent ba83586 commit d7f8115
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 7 deletions.
3 changes: 3 additions & 0 deletions decoder.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
27 changes: 20 additions & 7 deletions decoder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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) {
Expand All @@ -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)
}
})
}
}
Expand Down

0 comments on commit d7f8115

Please sign in to comment.