Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

decoder: add slice len bounds checks #6

Merged
merged 1 commit into from Aug 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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