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

SEC-29 Go zero values for missing struct fields in RLP decoding causes caller to panic #506

Closed
Gustav-Simonsson opened this issue Mar 16, 2015 · 5 comments
Assignees
Milestone

Comments

@Gustav-Simonsson
Copy link

https://github.com/ethereum/go-ethereum/blob/develop/rlp/decode.go#L343

If a field of a struct does not have a value in the decoded data, the decoder will set the field to the Go zero value for that field's type.

If it's a pointer, it's set to nil, which will cause a nil pointer deference later on.

This can be triggered remotely by sending an empty array as payload in a NewBlockMsg.

EDIT: As explained by @fjl the fix should be in the code calling the RLP decoder and not in the RLP decoder itself.

@fjl
Copy link
Contributor

fjl commented Mar 16, 2015

The RLP decoder is working as intended in this case. If an input list contains less than the reqired number of elements, the remaining struct fields are set to their zero value. This is documented.

The eth protocol layer should reject messages if the content is not valid. Not valid includes "has fields that are nil but shouldn't be".

@Gustav-Simonsson Gustav-Simonsson changed the title SEC-29 RLP Decoding DoS due to Go zero values for missing struct fields. SEC-29 Go zero values for missing struct fields in RLP decoding causes caller to panic Mar 17, 2015
@zelig zelig self-assigned this Mar 17, 2015
@zelig zelig added this to the Frontier milestone Mar 17, 2015
@zelig
Copy link
Contributor

zelig commented Mar 17, 2015

correct. I'll fix this

@obscuren
Copy link
Contributor

@zelig @fjl status?

@fjl
Copy link
Contributor

fjl commented Mar 27, 2015

@zelig will take care of adding more validation to the eth protocol. We have also discussed changing the decoder so it can never produce nil pointers unless they are actually wanted. This would be a bigger change.

@obscuren
Copy link
Contributor

obscuren commented Apr 1, 2015

#588

nolash pushed a commit to nolash/go-ethereum that referenced this issue May 8, 2018
…ters

swarm/storage: counters for errors and bytes for LazyChunkReader
maoueh pushed a commit to streamingfast/go-ethereum that referenced this issue Nov 12, 2021
maoueh pushed a commit to streamingfast/go-ethereum that referenced this issue Dec 9, 2022
…ice (ethereum#506)

internal/cli/server: update default cli values

Co-authored-by: SHIVAM SHARMA <shivam691999@gmail.com>
tanishqjasoria pushed a commit to tanishqjasoria/go-ethereum that referenced this issue Oct 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants