diff --git a/rlp/decode.go b/rlp/decode.go index 42be31a2d0..6bd13aa8fb 100644 --- a/rlp/decode.go +++ b/rlp/decode.go @@ -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: @@ -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 } @@ -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) @@ -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") @@ -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 } @@ -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 } @@ -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) } @@ -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) { @@ -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 @@ -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) { diff --git a/rlp/decode_test.go b/rlp/decode_test.go index e5c7f3761f..aa410de921 100644 --- a/rlp/decode_test.go +++ b/rlp/decode_test.go @@ -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)}, } @@ -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}, @@ -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}, @@ -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 != "" { + t.Log(test) t.Errorf("test %d: call %d (%s) unexpected error: %q", i, j, call, err) continue testfor } @@ -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}}, @@ -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"}, @@ -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), @@ -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)},