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

audit: Various Reentrancy Possibilities During Proposal Execution #192

Closed
Orland0x opened this issue Jun 13, 2023 · 1 comment · Fixed by #207
Closed

audit: Various Reentrancy Possibilities During Proposal Execution #192

Orland0x opened this issue Jun 13, 2023 · 1 comment · Fixed by #207
Assignees

Comments

@Orland0x
Copy link
Contributor

The execute function of the Space contract has a nonReentrant modifier, meaning the execution
strategy can't call back into it. However, it may still call all the other functions of the Space contract.
If external systems rely on the state of the Space contract to make decisions, this may lead to problems. For example, a malicious Space owner could execute a strategy which cancels its own proposal, which may lead an external system to believe the proposal can no longer be executed. However, the execute call is already being executed. Alternatively, a proposal author could update their proposal during its execution, or even vote on it.
For example, the author could execute the proposal while it is in the VotingAccepted state, and in the execution provide a high number of Against votes so that it is no longer accepted. An external system will again not be able to know that the proposal is already being executed, despite not having enough votes to pass.
These issues stem from the fact that the finalizationStatus of the proposal is only set to Executed after the execution of the proposal. If this status change were put in storage before the call (while still providing the same proposal in memory to the execution strategy), the reentrancies could be avoided as it would be clear that the execution of the proposal has already begun.

@pscott
Copy link
Collaborator

pscott commented Jun 16, 2023

FIX:
move finalizationStatus setting to before the call and cache the proposal

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.

2 participants