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

fix(dec-20-audit): [L11] Propose or Dispute a price from the zero address #2314

Merged
merged 1 commit into from
Dec 17, 2020

Conversation

daywiss
Copy link
Contributor

@daywiss daywiss commented Dec 16, 2020

Signed-off-by: David Adams david@umaproject.org

Motivation
The Optimistic oracle allows anyone to propose a price or dispute a proposal on behalf of the zero address. Since the state of the incentive mechanism is partially determined by whether the proposer or disputer addresses are non-zero, this won’t update the state machine, but it will set variables and emit events prematurely. For example, a client may incorrectly react to a DisputePrice event even though the dispute is not recognized by the oracle. Consider restricting the proposePriceFor and disputePriceFor function parameters to non-zero participants.

Summary

Adds 2 require statements in the appropriate functions and adds a tests to ensure calls fail when passing 0x0 address.

Signed-off-by: David Adams <david@umaproject.org>
Copy link
Member

@nicholaspai nicholaspai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Drive by: can you change the PR's title and the Motivation section of the description to copy exactly the audit's title and comments? Look at the other audit PR's for example.

@nicholaspai nicholaspai added the dec-2020-audit Fixes for the perpetual, optimistic oracle, ancillary DVM data and financial product library audit. label Dec 16, 2020
Copy link
Member

@nicholaspai nicholaspai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM nicely done

@coveralls
Copy link

Coverage Status

Coverage increased (+0.05%) to 93.211% when pulling dfdc72f on daywiss:L11 into 631504f on UMAprotocol:master.

@nicholaspai nicholaspai changed the title fix(dec-2020-audit): [L11] require non zero proposer/disputer address fix(dec-20-audit): [L11] require non zero proposer/disputer address Dec 16, 2020
Copy link
Member

@mrice32 mrice32 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a nice audit catch and good change. +1!

@daywiss daywiss changed the title fix(dec-20-audit): [L11] require non zero proposer/disputer address fix(dec-20-audit): [L11] Propose or Dispute a price from the zero address Dec 17, 2020
@daywiss daywiss merged commit e867fcd into UMAprotocol:master Dec 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dec-2020-audit Fixes for the perpetual, optimistic oracle, ancillary DVM data and financial product library audit.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants