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: resolution modules can only update the dispute's status forward #69

Open
wants to merge 1 commit into
base: fix/oz-audit
Choose a base branch
from

Conversation

xorsal
Copy link
Contributor

@xorsal xorsal commented Dec 10, 2024

🤖 Linear

Closes OPT-565

Copy link

linear bot commented Dec 10, 2024

@xorsal xorsal changed the base branch from dev to fix/oz-audit December 10, 2024 20:09
@@ -313,6 +313,11 @@ contract Oracle is IOracle, OracleAccessController {
if (msg.sender != _request.disputeModule && msg.sender != _request.resolutionModule) {
revert Oracle_NotDisputeOrResolutionModule(msg.sender);
}

if (msg.sender == _request.resolutionModule && _status <= DisputeStatus.Escalated) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Its true that <= is a very good solution to optimize, but what if we add new dispute status. IDK , for now is fine I guess

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Before adding a new state, we'd have to ensure the change is fully compatible with all existing modules and their respective flows. This will definitely be a pain point in future developments.

In this PR I specifically adhere to what the auditor found: a poorly implemented module could reverse the state of a dispute to a previous state, which is unexpected and could break an invariant (is it really an invariant? I don't think we have defined them yet)

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.

3 participants