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

rlp.inspect on recursive lists does not validate invalid list size? #254

Open
kdeme opened this issue Jun 18, 2020 · 1 comment
Open

rlp.inspect on recursive lists does not validate invalid list size? #254

kdeme opened this issue Jun 18, 2020 · 1 comment
Labels

Comments

@kdeme
Copy link
Contributor

kdeme commented Jun 18, 2020

Payloads with containing many rlp lists inside lists takes very long.
e.g. this payload ((it takes much less if the whole output part of the inspect is removed, but even without that this payload is >10s):

c5c2c1c3c5c2c1c5c2c5c2c1c5c3c5c2c1c3c5c5c2c5c2c1c5c3c1c3c5c2
c1c5c2c5c2c1c5c2c530c1c5c2c5c2c1c5c3c1c3c5c2c1c5c2c5c2c1c5c3
c5c2c1c3c2c1c3c5c2c1c5c2c5c2c1c5c3c5c2c1c3c5c2c530c1c5c1c5c2
c352c545c230c5c2c187c5c2c1423f3a3a3dc5

However, this payload is also incorrect, as several sizes of lists are not valid.
So rlp.inspect does not seem to validate the list sizes.

rlp.inspect is not used except for testing, so perhaps it is not really an issue.

@jlokier
Copy link
Contributor

jlokier commented Jul 29, 2021

I suspect this issue affects RLP parsing in general, not just rlp.inspect, and that it's a few different vulnerabilities when parsing untrusted RLP data from the network:

  • Iterating over the items in an RLP list doesn't check that the last item ends at the end of the containing list, so the last item in the list can overlap the item following the list.
  • Iterating over the fields of an object doesn't check that the object ends at the end of the containing list.
  • Iterating over a list sometimes does one scan to count the number of items, then another to process the items.
    With enough nesting could result in quadratic time behaviour.
  • There's a lot of repeated analysis of the first bytes of an item.
  • There's a lot of copying. Compined with quadratic time, this might explain the slow rlp.inspect on your payload.
  • Sometimes .blobLen > 0 is used to decide whether to parse an optional value or skip the value as "none". But this unintentionally treats a list as "none" instead of an error.

A number of places need to treat blobs like list containers as well, i.e. blobs whose content is itself RLP encoded. So I think a bit of reworking how nested RLP structures are parsed is in order, to validate boundaries more carefully (end reached exactly without overrun).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants