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

Beacon operation validation can raise IndexErrors #4

Open
gnattishness opened this issue Jan 23, 2020 · 3 comments
Open

Beacon operation validation can raise IndexErrors #4

gnattishness opened this issue Jan 23, 2020 · 3 comments

Comments

@gnattishness
Copy link
Contributor

What is wrong?

Several block operation validation functions can raise IndexError in addition to a ValidationError e.g. validate_voluntary_exit(), validate_proposer_slashing() when passed an Operation with an invalid validator index.

It is not obvious whether this is a documentation issue (that IndexErrors are an expected result), or a crash-causing bug and DOS vector:

How can it be fixed

Catch IndexErrors immediately and wrap them with a ValidationError. This is clearer and avoids potential bugs from forgetting to catch IndexErrors.

How it was found

Discovered via beacon-fuzz (initial testing of proposer_slashing fuzzer).
Triggering case: proposer_slashing-crash-35a9d8e810ef1c20f057ee4e6aa8a927dc2ed6dc with the following beacon_states
Or the pre-processed input propslash_preprocessed_indexerror.ssz can be directly passed to the trinity harness

@ChihChengLiang
Copy link
Contributor

Thank you @gnattishness for reporting this. Made a pull request to fix it. ethereum/trinity#1502

@ChihChengLiang
Copy link
Contributor

Sorry I just saw ethereum/trinity#1498 🙈

@ralexstokes ralexstokes transferred this issue from ethereum/trinity Aug 25, 2020
@ralexstokes
Copy link
Member

Refer this PR: ethereum/trinity#1498

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 a pull request may close this issue.

3 participants