code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/policies/Governance.sol#L205-L289

Vulnerability details

Impact

It may effect the fairness of the voting process

Proof of Concept

1) activateProposal There can be only one active proposal at a time. Therefore, if alice frontruns bob's activateProposal, the bob should wait until the GRACE_PERIOD. By doing so, one can prevent somebody else's proposal to go active.

2) vote The vote does not contain which proposalId to vote for/against as parameter. This may make it possible for frontrun the vote by activateProposal so that the vote was counted for the propose, which the voter was not intended. Or the miners may hold the vote back to use it for other proposal. This is especially problem, because even after the GRACE_PERIOD one can vote, and the executeProposal can be called for the proposal. At the same time, a new proposal can be activated.

3) executeProposal The executeProposal will be successful if 1) the vote passed 2) after EXECUTION_TIMELOCK. Therefore, after the EXECUTIOIN_TIMELOCK, it is open for votes but one can execute as soon as the number of votes meets the condition. So, it gives an opportunity to frontrun against vote to execute before the against votes are count.

Tools Used

none

Recommended Mitigation Steps

The vote function should contain proposalId for the vote and compared to the active proposal. It is good to have a fixed window of votes, and allow to vote only within this window. Then count the votes after the voting window and execute the proposal if it passed. Currently there are overraps between voting, executing, deactivating the current proposal and activating the next proposal, which introduces frontrun opportunities.

bahurum commented 2 years ago

Point no 2. is Duplicate of #273

fullyallocated commented 2 years ago

This is how the voting system is currently intended to work—it's first come first serve after the endorsement threshold is passed. While not the best mechanism, it is the intended behavior.

0xean commented 2 years ago

Going to mark as a duplicate of #273 - although there are additional points beyond #273 , all have been covered in other issues.

code-423n4 / 2022-08-olympus-findings