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

Exploit allowing votes on mis-matched epoch/hash #57

Closed
djrtwo opened this Issue Mar 23, 2018 · 1 comment

Comments

Projects
None yet
1 participant
@djrtwo
Contributor

djrtwo commented Mar 23, 2018

Issue

This was found by the Runtime Verification team in and initial audit of the contract. Due to an incorrect sequence of asserts, a user could presumably cast a vote for {epoch: n-1, hash: hash(epochN)}.

Below are the details copied from Daejun Park:
The vote method doesn't check the consistency between target_hash and target_epoch. On line 390, it asserts target_hash == self.get_recommended_target_hash(). Why not assert self.current_epoch == target_epoch as well?

Suppose current_epoch is n, get_recommended_target_hash() returns Tn, which is blockhash(n*epoch_length -1) and expected_source_epoch returns Sn. A validator with index Vi prepares a vote message like (Vi, Tn , n-1, Sn).

I run the vote message manually on the vote method:

  1. It passed the check on line 387, line 390 , line 392 and line 404.
  2. On line 413 and 416, the vote is accounted as a valid vote and added to the votes in epoch n-1!
  3. The validator will not get any reward on line 421.
  4. It may cause epoch n-1 to be justified on line 429, even though current epoch is n.

Proposed Implementation

  • Codify the above exploit in a test showing that it is in fact possible
  • Patch the casper vote method asserting that target_epoch must be current_epoch
  • Remove other if statements in the vote method related to target_epoch == current_epoch because was already asserted at head of method.
assert target_hash == self.get_recommended_target_hash()
assert target_epoch == self.current_epoch
@djrtwo

This comment has been minimized.

Contributor

djrtwo commented Mar 29, 2018

fixed in #58

@djrtwo djrtwo closed this Mar 29, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment