Skip to content

Commit

Permalink
rlp: stricter validation of canonical integer format
Browse files Browse the repository at this point in the history 
All integers (including size information in type tags) need to be
encoded using the smallest possible encoding. This commit expands the
stricter validation introduced for *big.Int in commit 59597d2
to all integer types and size tags.
  • Loading branch information
@fjl
fjl committed Apr 17, 2015
1 parent 6788f95 commit 6e9f803
Show file tree
Hide file tree
Showing 2 changed files with 93 additions and 42 deletions.
97 changes: 63 additions & 34 deletions rlp/decode.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,9 @@ func (err *decodeError) Error() string {
func wrapStreamError(err error, typ reflect.Type) error {
switch err {
case ErrCanonInt:
return &decodeError{msg: "canon int error appends zero's", typ: typ}
return &decodeError{msg: "non-canonical integer (leading zero bytes)", typ: typ}
case ErrCanonSize:
return &decodeError{msg: "non-canonical size information", typ: typ}
case ErrExpectedList:
return &decodeError{msg: "expected input list", typ: typ}
case ErrExpectedString:
Expand Down Expand Up @@ -195,12 +197,10 @@ func decodeBigInt(s *Stream, val reflect.Value) error {
i = new(big.Int)
val.Set(reflect.ValueOf(i))
}

// Reject big integers which are zero appended
// Reject leading zero bytes
if len(b) > 0 && b[0] == 0 {
return wrapStreamError(ErrCanonInt, val.Type())
}

i.SetBytes(b)
return nil
}
Expand Down Expand Up @@ -270,7 +270,7 @@ func decodeListSlice(s *Stream, val reflect.Value, elemdec decoder) error {
func decodeListArray(s *Stream, val reflect.Value, elemdec decoder) error {
size, err := s.List()
if err != nil {
return err
return wrapStreamError(err, val.Type())
}
if size == 0 {
zero(val, 0)
Expand Down Expand Up @@ -474,10 +474,11 @@ var (
// has been reached during streaming.
EOL = errors.New("rlp: end of list")

// Other errors
// Actual Errors
ErrExpectedString = errors.New("rlp: expected String or Byte")
ErrExpectedList = errors.New("rlp: expected List")
ErrCanonInt = errors.New("rlp: expected Int")
ErrCanonInt = errors.New("rlp: non-canonical (leading zero bytes) integer")
ErrCanonSize = errors.New("rlp: non-canonical size information")
ErrElemTooLarge = errors.New("rlp: element is larger than containing list")
ErrValueTooLarge = errors.New("rlp: value size exceeds available input length")

Expand Down Expand Up @@ -519,6 +520,7 @@ type Stream struct {
kind Kind // kind of value ahead
size uint64 // size of value ahead
byteval byte // value of single byte in type tag
kinderr error // error from last readKind
stack []listpos
}

Expand Down Expand Up @@ -619,13 +621,21 @@ func (s *Stream) uint(maxbits int) (uint64, error) {
}
switch kind {
case Byte:
if s.byteval == 0 {
return 0, ErrCanonInt
}
s.kind = -1 // rearm Kind
return uint64(s.byteval), nil
case String:
if size > uint64(maxbits/8) {
return 0, errUintOverflow
}
return s.readUint(byte(size))
v, err := s.readUint(byte(size))
if err == ErrCanonSize {
// Adjust error because we're not reading a size right now.
err = ErrCanonInt
}
return v, err
default:
return 0, ErrExpectedString
}
Expand Down Expand Up @@ -729,6 +739,7 @@ func (s *Stream) Reset(r io.Reader, inputLimit uint64) {
s.stack = s.stack[:0]
s.size = 0
s.kind = -1
s.kinderr = nil
if s.uintbuf == nil {
s.uintbuf = make([]byte, 8)
}
Expand All @@ -751,32 +762,31 @@ func (s *Stream) Kind() (kind Kind, size uint64, err error) {
tos = &s.stack[len(s.stack)-1]
}
if s.kind < 0 {
s.kinderr = nil
// Don't read further if we're at the end of the
// innermost list.
if tos != nil && tos.pos == tos.size {
return 0, 0, EOL
}
kind, size, err = s.readKind()
if err != nil {
return 0, 0, err
}
s.kind, s.size = kind, size
}
// Make sure size is reasonable. This is done always
// so Kind returns the same error when called multiple times.
if tos == nil {
// At toplevel, check that the value is smaller
// than the remaining input length.
if s.limited && s.size > s.remaining {
return 0, 0, ErrValueTooLarge
}
} else {
// Inside a list, check that the value doesn't overflow the list.
if s.size > tos.size-tos.pos {
return 0, 0, ErrElemTooLarge
s.kind, s.size, s.kinderr = s.readKind()
if s.kinderr == nil {
if tos == nil {
// At toplevel, check that the value is smaller
// than the remaining input length.
if s.limited && s.size > s.remaining {
s.kinderr = ErrValueTooLarge
}
} else {
// Inside a list, check that the value doesn't overflow the list.
if s.size > tos.size-tos.pos {
s.kinderr = ErrElemTooLarge
}
}
}
}
return s.kind, s.size, nil
// Note: this might return a sticky error generated
// by an earlier call to readKind.
return s.kind, s.size, s.kinderr
}

func (s *Stream) readKind() (kind Kind, size uint64, err error) {
Expand Down Expand Up @@ -805,6 +815,9 @@ func (s *Stream) readKind() (kind Kind, size uint64, err error) {
// would be encoded as 0xB90400 followed by the string. The range of
// the first byte is thus [0xB8, 0xBF].
size, err = s.readUint(b - 0xB7)
if err == nil && size < 56 {
err = ErrCanonSize
}
return String, size, err
case b < 0xF8:
// If the total payload of a list
Expand All @@ -821,24 +834,40 @@ func (s *Stream) readKind() (kind Kind, size uint64, err error) {
// the concatenation of the RLP encodings of the items. The
// range of the first byte is thus [0xF8, 0xFF].
size, err = s.readUint(b - 0xF7)
if err == nil && size < 56 {
err = ErrCanonSize
}
return List, size, err
}
}

func (s *Stream) readUint(size byte) (uint64, error) {
if size == 1 {
switch size {
case 0:
s.kind = -1 // rearm Kind
return 0, nil
case 1:
b, err := s.readByte()
if err == io.EOF {
err = io.ErrUnexpectedEOF
}
return uint64(b), err
default:
start := int(8 - size)
for i := 0; i < start; i++ {
s.uintbuf[i] = 0
}
if err := s.readFull(s.uintbuf[start:]); err != nil {
return 0, err
}
if s.uintbuf[start] == 0 {
// Note: readUint is also used to decode integer
// values. The error needs to be adjusted to become
// ErrCanonInt in this case.
return 0, ErrCanonSize
}
return binary.BigEndian.Uint64(s.uintbuf), nil
}
start := int(8 - size)
for i := 0; i < start; i++ {
s.uintbuf[i] = 0
}
err := s.readFull(s.uintbuf[start:])
return binary.BigEndian.Uint64(s.uintbuf), err
}

func (s *Stream) readFull(buf []byte) (err error) {
Expand Down
38 changes: 30 additions & 8 deletions rlp/decode_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,11 @@ func TestStreamKind(t *testing.T) {
{"7F", Byte, 0},
{"80", String, 0},
{"B7", String, 55},
{"B800", String, 0},
{"B90400", String, 1024},
{"BA000400", String, 1024},
{"BB00000400", String, 1024},
{"BFFFFFFFFFFFFFFFFF", String, ^uint64(0)},
{"C0", List, 0},
{"C8", List, 8},
{"F7", List, 55},
{"F800", List, 0},
{"F804", List, 4},
{"F90400", List, 1024},
{"FFFFFFFFFFFFFFFFFF", List, ^uint64(0)},
}
Expand Down Expand Up @@ -85,7 +80,7 @@ func TestStreamErrors(t *testing.T) {
string
calls
newStream func([]byte) *Stream // uses bytes.Reader if nil
error
error error
}{
{"C0", calls{"Bytes"}, nil, ErrExpectedString},
{"C0", calls{"Uint"}, nil, ErrExpectedString},
Expand All @@ -98,6 +93,21 @@ func TestStreamErrors(t *testing.T) {
{"00", calls{"ListEnd"}, nil, errNotInList},
{"C401020304", calls{"List", "Uint", "ListEnd"}, nil, errNotAtEOL},

// Leading zero bytes are rejected when reading integers.
{"00", calls{"Uint"}, nil, ErrCanonInt},
{"820002", calls{"Uint"}, nil, ErrCanonInt},

// Size tags must use the smallest possible encoding.
// Leading zero bytes in the size tag are also rejected.
{"B800", calls{"Kind"}, withoutInputLimit, ErrCanonSize},
{"B90000", calls{"Kind"}, withoutInputLimit, ErrCanonSize},
{"B90055", calls{"Kind"}, withoutInputLimit, ErrCanonSize},
{"BA0002FFFF", calls{"Bytes"}, withoutInputLimit, ErrCanonSize},
{"F800", calls{"Kind"}, withoutInputLimit, ErrCanonSize},
{"F90000", calls{"Kind"}, withoutInputLimit, ErrCanonSize},
{"F90055", calls{"Kind"}, withoutInputLimit, ErrCanonSize},
{"FA0002FFFF", calls{"List"}, withoutInputLimit, ErrCanonSize},

// Expected EOF
{"", calls{"Kind"}, nil, io.EOF},
{"", calls{"Uint"}, nil, io.EOF},
Expand Down Expand Up @@ -170,10 +180,12 @@ testfor:
want = test.error.Error()
}
if err != want {
t.Log(test)
t.Errorf("test %d: last call (%s) error mismatch\ngot: %s\nwant: %s",
i, call, err, test.error)
}
} else if err != "<nil>" {
t.Log(test)
t.Errorf("test %d: call %d (%s) unexpected error: %q", i, j, call, err)
continue testfor
}
Expand Down Expand Up @@ -289,15 +301,20 @@ var decodeTests = []decodeTest{
{input: "8405050505", ptr: new(uint32), value: uint32(0x05050505)},
{input: "850505050505", ptr: new(uint32), error: "rlp: input string too long for uint32"},
{input: "C0", ptr: new(uint32), error: "rlp: expected input string or byte for uint32"},
{input: "00", ptr: new(uint32), error: "rlp: non-canonical integer (leading zero bytes) for uint32"},
{input: "820004", ptr: new(uint32), error: "rlp: non-canonical integer (leading zero bytes) for uint32"},
{input: "B8020004", ptr: new(uint32), error: "rlp: non-canonical size information for uint32"},

// slices
{input: "C0", ptr: new([]uint), value: []uint{}},
{input: "C80102030405060708", ptr: new([]uint), value: []uint{1, 2, 3, 4, 5, 6, 7, 8}},
{input: "F8020004", ptr: new([]uint), error: "rlp: non-canonical size information for []uint"},

// arrays
{input: "C0", ptr: new([5]uint), value: [5]uint{}},
{input: "C50102030405", ptr: new([5]uint), value: [5]uint{1, 2, 3, 4, 5}},
{input: "C6010203040506", ptr: new([5]uint), error: "rlp: input list has too many elements for [5]uint"},
{input: "F8020004", ptr: new([5]uint), error: "rlp: non-canonical size information for [5]uint"},

// byte slices
{input: "01", ptr: new([]byte), value: []byte{1}},
Expand Down Expand Up @@ -352,7 +369,7 @@ var decodeTests = []decodeTest{
// big ints
{input: "01", ptr: new(*big.Int), value: big.NewInt(1)},
{input: "89FFFFFFFFFFFFFFFFFF", ptr: new(*big.Int), value: veryBigInt},
{input: "820001", ptr: new(big.Int), error: "rlp: canon int error appends zero's for *big.Int"},
{input: "820001", ptr: new(big.Int), error: "rlp: non-canonical integer (leading zero bytes) for *big.Int"},
{input: "10", ptr: new(big.Int), value: *big.NewInt(16)}, // non-pointer also works
{input: "C0", ptr: new(*big.Int), error: "rlp: expected input string or byte for *big.Int"},

Expand All @@ -366,6 +383,11 @@ var decodeTests = []decodeTest{
value: recstruct{1, &recstruct{2, &recstruct{3, nil}}},
},

{
input: "83222222",
ptr: new(simplestruct),
error: "rlp: expected input list for rlp.simplestruct",
},
{
input: "C3010101",
ptr: new(simplestruct),
Expand All @@ -378,7 +400,7 @@ var decodeTests = []decodeTest{
},

// pointers
{input: "00", ptr: new(*uint), value: uintp(0)},
{input: "00", ptr: new(*[]byte), value: &[]byte{0}},
{input: "80", ptr: new(*uint), value: (*uint)(nil)},
{input: "C0", ptr: new(*uint), value: (*uint)(nil)},
{input: "07", ptr: new(*uint), value: uintp(7)},
Expand Down

0 comments on commit 6e9f803

Please sign in to comment.