Skip to content

Commit

Permalink
Merge pull request #6 from terorie/decoder-slice-bounds
Browse files Browse the repository at this point in the history
decoder: add slice len bounds checks
  • Loading branch information
gagliardetto committed Aug 20, 2022
2 parents 990fbf1 + bbaf72e commit 6096313
Show file tree
Hide file tree
Showing 6 changed files with 37 additions and 10 deletions.
6 changes: 6 additions & 0 deletions decoder.go
Expand Up @@ -245,12 +245,18 @@ func (dec *Decoder) ReadLength() (length int, err error) {
if err != nil {
return 0, err
}
if val > 0x7FFF_FFFF {
return 0, io.ErrUnexpectedEOF
}
length = int(val)
case EncodingBorsh:
val, err := dec.ReadUint32(LE)
if err != nil {
return 0, err
}
if val > 0x7FFF_FFFF {
return 0, io.ErrUnexpectedEOF
}
length = int(val)
case EncodingCompactU16:
val, err := DecodeCompactU16LengthFromByteReader(dec)
Expand Down
13 changes: 8 additions & 5 deletions decoder_bin.go
Expand Up @@ -20,6 +20,7 @@ package bin
import (
"encoding/binary"
"fmt"
"io"
"reflect"

"go.uber.org/zap"
Expand Down Expand Up @@ -169,18 +170,21 @@ func (dec *Decoder) decodeBin(rv reflect.Value, opt *option) (err error) {
if opt.hasSizeOfSlice() {
l = opt.getSizeOfSlice()
} else {
// TODO: what type is length? Is it really Uvarint64?
length, err := dec.ReadUvarint64()
length, err := dec.ReadLength()
if err != nil {
return err
}
l = int(length)
l = length
}

if traceEnabled {
zlog.Debug("reading slice", zap.Int("len", l), typeField("type", rv))
}

if l > dec.Remaining() {
return io.ErrUnexpectedEOF
}

rv.Set(reflect.MakeSlice(rt, l, l))
for i := 0; i < l; i++ {
if err = dec.decodeBin(rv.Index(i), nil); err != nil {
Expand All @@ -194,8 +198,7 @@ func (dec *Decoder) decodeBin(rv reflect.Value, opt *option) (err error) {
}

case reflect.Map:
// TODO: what type is length? Is it really Uvarint64?
l, err := dec.ReadUvarint64()
l, err := dec.ReadLength()
if err != nil {
return err
}
Expand Down
4 changes: 4 additions & 0 deletions decoder_borsh.go
Expand Up @@ -20,6 +20,7 @@ package bin
import (
"errors"
"fmt"
"io"
"reflect"

"go.uber.org/zap"
Expand Down Expand Up @@ -200,6 +201,9 @@ func (dec *Decoder) decodeBorsh(rv reflect.Value, opt *option) (err error) {
// Empty slices are left nil
return
}
if l > dec.Remaining() {
return io.ErrUnexpectedEOF
}

rv.Set(reflect.MakeSlice(rt, l, l))
for i := 0; i < l; i++ {
Expand Down
5 changes: 5 additions & 0 deletions decoder_compact-u16.go
Expand Up @@ -19,6 +19,7 @@ package bin

import (
"fmt"
"io"
"reflect"

"go.uber.org/zap"
Expand Down Expand Up @@ -179,6 +180,10 @@ func (dec *Decoder) decodeCompactU16(rv reflect.Value, opt *option) (err error)
zlog.Debug("reading slice", zap.Int("len", l), typeField("type", rv))
}

if l > dec.Remaining() {
return io.ErrUnexpectedEOF
}

rv.Set(reflect.MakeSlice(rt, l, l))
for i := 0; i < l; i++ {
if err = dec.decodeCompactU16(rv.Index(i), nil); err != nil {
Expand Down
11 changes: 10 additions & 1 deletion decoder_test.go
Expand Up @@ -599,7 +599,16 @@ func TestDecoder_Slice_Err(t *testing.T) {

decoder = NewBinDecoder(buf)
err = decoder.Decode(&s)
assert.EqualError(t, err, "decode: uint64 required [8] bytes, remaining [0]")
assert.EqualError(t, err, "unexpected EOF")
}

func TestDecoder_Slice_InvalidLen(t *testing.T) {
buf := []byte{0xd7, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x01}

decoder := NewBinDecoder(buf)
var s []string
err := decoder.Decode(&s)
assert.EqualError(t, err, "unexpected EOF")
}

func TestDecoder_Int64(t *testing.T) {
Expand Down
8 changes: 4 additions & 4 deletions variant.go
Expand Up @@ -147,10 +147,10 @@ var NoTypeIDDefaultID = TypeIDFromUint8(0)

// NewVariantDefinition creates a variant definition based on the *ordered* provided types.
//
// - For anchor instructions, it's the name that defines the binary variant value.
// - For all other types, it's the ordering that defines the binary variant value just like in native `nodeos` C++
// and in Smart Contract via the `std::variant` type. It's important to pass the entries
// in the right order!
// - For anchor instructions, it's the name that defines the binary variant value.
// - For all other types, it's the ordering that defines the binary variant value just like in native `nodeos` C++
// and in Smart Contract via the `std::variant` type. It's important to pass the entries
// in the right order!
//
// This variant definition can now be passed to functions of `BaseVariant` to implement
// marshal/unmarshaling functionalities for binary & JSON.
Expand Down

0 comments on commit 6096313

Please sign in to comment.