Skip to content

Commit

Permalink
rlp: fix nil pointer decoding
Browse files Browse the repository at this point in the history
The generic pointer decoder did not advance the input position
for empty values. This can lead to strange issues and even
infinite loops.
  • Loading branch information
fjl committed Mar 20, 2015
1 parent 28ddc16 commit b41185a
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 2 deletions.
7 changes: 6 additions & 1 deletion rlp/decode.go
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,12 @@ func makePtrDecoder(typ reflect.Type) (decoder, error) {
dec := func(s *Stream, val reflect.Value) (err error) {
_, size, err := s.Kind()
if err != nil || size == 0 && s.byteval == 0 {
val.Set(reflect.Zero(typ)) // set to nil
// rearm s.Kind. This is important because the input
// position must advance to the next value even though
// we don't read anything.
s.kind = -1
// set the pointer to nil.
val.Set(reflect.Zero(typ))
return err
}
newval := val
Expand Down
22 changes: 21 additions & 1 deletion rlp/decode_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func TestStreamKind(t *testing.T) {
s := NewStream(bytes.NewReader(unhex(test.input)))
kind, len, err := s.Kind()
if err != nil {
t.Errorf("test %d: Type returned error: %v", i, err)
t.Errorf("test %d: Kind returned error: %v", i, err)
continue
}
if kind != test.wantKind {
Expand Down Expand Up @@ -93,6 +93,23 @@ func TestStreamErrors(t *testing.T) {
{"C3C2010201", calls{"List", "List", "Uint", "Uint", "ListEnd", "Uint"}, EOL},
{"00", calls{"ListEnd"}, errNotInList},
{"C40102", calls{"List", "Uint", "ListEnd"}, errNotAtEOL},

// This test verifies that the input position is advanced
// correctly when calling Bytes for empty strings. Kind can be called
// any number of times in between and doesn't advance.
{"C3808080", calls{
"List", // enter the list
"Bytes", // past first element

"Kind", "Kind", "Kind", // this shouldn't advance

"Bytes", // past second element

"Kind", "Kind", // can't hurt to try

"Bytes", // past final element
"Bytes", // this one should fail
}, EOL},
}

testfor:
Expand Down Expand Up @@ -314,6 +331,9 @@ var decodeTests = []decodeTest{
{input: "C109", ptr: new(*[]uint), value: &[]uint{9}},
{input: "C58403030303", ptr: new(*[][]byte), value: &[][]byte{{3, 3, 3, 3}}},

// check that input position is advanced also empty values.
{input: "C3808005", ptr: new([]*uint), value: []*uint{nil, nil, uintp(5)}},

// pointer should be reset to nil
{input: "05", ptr: sharedPtr, value: uintp(5)},
{input: "80", ptr: sharedPtr, value: (*uint)(nil)},
Expand Down

0 comments on commit b41185a

Please sign in to comment.