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

Add more RLP invalid tests for non-optimal lengths. #612

Merged
merged 1 commit into from
Jul 22, 2019

Conversation

acoglio
Copy link
Member

@acoglio acoglio commented Jul 18, 2019

The pre-existing tests bytesShouldBeSingleByteXX cover the case in which a
single byte below 128 is erroneously encoded with a one-byte (i.e. short)
length.

The new proposed tests cover the following additional cases:

  • A multi-byte (i.e. long) length has leading zeros, for byte arrays or for
    lists.
  • A multi-byte length is used where a short length suffices, for byte arrays and
    for lists.

I tried running the scripts that check the well-formedness of the tests, but ran into problems and created Issue #611. Also, I'm not sure which other requirements should be fulfilled in order for these new tests to be accepted.

To provide some assurance, the new proposed tests are rejected by the formally verified RLP decoder available at https://github.com/acl2/acl2/blob/master/books/kestrel/ethereum/rlp/decoding-executable.lisp.

The pre-existing tests bytesShouldBeSingleByteXX cover the case in which a
single byte below 128 is erroneously encoded with a one-byte (i.e. short)
length.

The new proposed tests cover the following additional cases:

- A multi-byte (i.e. long) length has leading zeros, for byte arrays or for
  lists.

- A multi-byte length is used where a short length suffices, for byte arrays and
  for lists.
@winsvega winsvega merged commit 2f23783 into ethereum:develop Jul 22, 2019
@acoglio acoglio deleted the rlp-quasi-encodings branch July 23, 2019 01:36
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

Successfully merging this pull request may close these issues.

2 participants