From 1e2c93aa2da453ef9548b9957b5ed453f60ce5ca Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Thu, 16 Apr 2015 10:15:14 +0200 Subject: [PATCH] rlp: reject non-minimal input strings Input strings of length 1 containing a byte < 56 are non-minimal and should be encoded as a single byte instead. Reject such strings. --- rlp/decode.go | 28 +++++++++++++++++++++------- rlp/decode_test.go | 32 ++++++++++++++++++++++++-------- 2 files changed, 45 insertions(+), 15 deletions(-) diff --git a/rlp/decode.go b/rlp/decode.go index 6bd13aa8fb..43dd716b54 100644 --- a/rlp/decode.go +++ b/rlp/decode.go @@ -305,10 +305,11 @@ func decodeByteSlice(s *Stream, val reflect.Value) error { return decodeListSlice(s, val, decodeUint) } b, err := s.Bytes() - if err == nil { - val.SetBytes(b) + if err != nil { + return wrapStreamError(err, val.Type()) } - return err + val.SetBytes(b) + return nil } func decodeByteArray(s *Stream, val reflect.Value) error { @@ -333,6 +334,10 @@ func decodeByteArray(s *Stream, val reflect.Value) error { return err } zero(val, int(size)) + // Reject cases where single byte encoding should have been used. + if size == 1 && slice[0] < 56 { + return wrapStreamError(ErrCanonSize, val.Type()) + } case List: return decodeListArray(s, val, decodeUint) } @@ -477,7 +482,7 @@ var ( // Actual Errors ErrExpectedString = errors.New("rlp: expected String or Byte") ErrExpectedList = errors.New("rlp: expected List") - ErrCanonInt = errors.New("rlp: non-canonical (leading zero bytes) integer") + ErrCanonInt = errors.New("rlp: non-canonical integer format") 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") @@ -576,6 +581,9 @@ func (s *Stream) Bytes() ([]byte, error) { if err = s.readFull(b); err != nil { return nil, err } + if size == 1 && b[0] < 56 { + return nil, ErrCanonSize + } return b, nil default: return nil, ErrExpectedString @@ -631,11 +639,17 @@ func (s *Stream) uint(maxbits int) (uint64, error) { return 0, errUintOverflow } v, err := s.readUint(byte(size)) - if err == ErrCanonSize { + switch { + case err == ErrCanonSize: // Adjust error because we're not reading a size right now. - err = ErrCanonInt + return 0, ErrCanonInt + case err != nil: + return 0, err + case size > 0 && v < 56: + return 0, ErrCanonSize + default: + return v, nil } - return v, err default: return 0, ErrExpectedString } diff --git a/rlp/decode_test.go b/rlp/decode_test.go index aa410de921..7e2ea2041b 100644 --- a/rlp/decode_test.go +++ b/rlp/decode_test.go @@ -93,12 +93,16 @@ 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. + // Non-canonical integers (e.g. leading zero bytes). {"00", calls{"Uint"}, nil, ErrCanonInt}, {"820002", calls{"Uint"}, nil, ErrCanonInt}, + {"8133", calls{"Uint"}, nil, ErrCanonSize}, + {"8156", calls{"Uint"}, nil, nil}, // Size tags must use the smallest possible encoding. // Leading zero bytes in the size tag are also rejected. + {"8100", calls{"Uint"}, nil, ErrCanonSize}, + {"8100", calls{"Bytes"}, nil, ErrCanonSize}, {"B800", calls{"Kind"}, withoutInputLimit, ErrCanonSize}, {"B90000", calls{"Kind"}, withoutInputLimit, ErrCanonSize}, {"B90055", calls{"Kind"}, withoutInputLimit, ErrCanonSize}, @@ -112,7 +116,7 @@ func TestStreamErrors(t *testing.T) { {"", calls{"Kind"}, nil, io.EOF}, {"", calls{"Uint"}, nil, io.EOF}, {"", calls{"List"}, nil, io.EOF}, - {"8105", calls{"Uint", "Uint"}, nil, io.EOF}, + {"8158", calls{"Uint", "Uint"}, nil, io.EOF}, {"C0", calls{"List", "ListEnd", "List"}, nil, io.EOF}, // Input limit errors. @@ -129,11 +133,11 @@ func TestStreamErrors(t *testing.T) { // down toward zero in Stream.remaining, reading too far can overflow // remaining to a large value, effectively disabling the limit. {"C40102030401", calls{"Raw", "Uint"}, withCustomInputLimit(5), io.EOF}, - {"C4010203048102", calls{"Raw", "Uint"}, withCustomInputLimit(6), ErrValueTooLarge}, + {"C4010203048158", calls{"Raw", "Uint"}, withCustomInputLimit(6), ErrValueTooLarge}, // Check that the same calls are fine without a limit. {"C40102030401", calls{"Raw", "Uint"}, withoutInputLimit, nil}, - {"C4010203048102", calls{"Raw", "Uint"}, withoutInputLimit, nil}, + {"C4010203048158", calls{"Raw", "Uint"}, withoutInputLimit, nil}, // Unexpected EOF. This only happens when there is // no input limit, so the reader needs to be 'dumbed down'. @@ -295,13 +299,13 @@ var decodeTests = []decodeTest{ // integers {input: "05", ptr: new(uint32), value: uint32(5)}, {input: "80", ptr: new(uint32), value: uint32(0)}, - {input: "8105", ptr: new(uint32), value: uint32(5)}, {input: "820505", ptr: new(uint32), value: uint32(0x0505)}, {input: "83050505", ptr: new(uint32), value: uint32(0x050505)}, {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: "8105", ptr: new(uint32), error: "rlp: non-canonical size information 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"}, @@ -319,10 +323,16 @@ var decodeTests = []decodeTest{ // byte slices {input: "01", ptr: new([]byte), value: []byte{1}}, {input: "80", ptr: new([]byte), value: []byte{}}, + {input: "8D6162636465666768696A6B6C6D", ptr: new([]byte), value: []byte("abcdefghijklm")}, {input: "C0", ptr: new([]byte), value: []byte{}}, {input: "C3010203", ptr: new([]byte), value: []byte{1, 2, 3}}, + { + input: "8105", + ptr: new([]byte), + error: "rlp: non-canonical size information for []uint8", + }, { input: "C3820102", ptr: new([]byte), @@ -346,10 +356,15 @@ var decodeTests = []decodeTest{ ptr: new([5]byte), error: "rlp: input string too long for [5]uint8", }, + { + input: "8105", + ptr: new([5]byte), + error: "rlp: non-canonical size information for [5]uint8", + }, // byte array reuse (should be zeroed) {input: "850102030405", ptr: &sharedByteArray, value: [5]byte{1, 2, 3, 4, 5}}, - {input: "8101", ptr: &sharedByteArray, value: [5]byte{1}}, // kind: String + {input: "01", ptr: &sharedByteArray, value: [5]byte{1}}, // kind: String {input: "850102030405", ptr: &sharedByteArray, value: [5]byte{1, 2, 3, 4, 5}}, {input: "01", ptr: &sharedByteArray, value: [5]byte{1}}, // kind: Byte {input: "C3010203", ptr: &sharedByteArray, value: [5]byte{1, 2, 3, 0, 0}}, @@ -369,9 +384,10 @@ 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: 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"}, + {input: "820001", ptr: new(big.Int), error: "rlp: non-canonical integer (leading zero bytes) for *big.Int"}, + {input: "8105", ptr: new(big.Int), error: "rlp: non-canonical size information for *big.Int"}, // structs {input: "C0", ptr: new(simplestruct), value: simplestruct{0, ""}}, @@ -404,7 +420,7 @@ var decodeTests = []decodeTest{ {input: "80", ptr: new(*uint), value: (*uint)(nil)}, {input: "C0", ptr: new(*uint), value: (*uint)(nil)}, {input: "07", ptr: new(*uint), value: uintp(7)}, - {input: "8108", ptr: new(*uint), value: uintp(8)}, + {input: "8158", ptr: new(*uint), value: uintp(0x58)}, {input: "C109", ptr: new(*[]uint), value: &[]uint{9}}, {input: "C58403030303", ptr: new(*[][]byte), value: &[][]byte{{3, 3, 3, 3}}},