code423n4 commented 2 years ago

Lines of code

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

Vulnerability details

Impact

It may cause a deadlock situation

Condition:

there is no proposal with enough endorsement
majority (depending on the endorsement threshold) of votes are locked in the current proposal
The votes for the current proposal are balanced between for and against so it does not meet the criteria to be executed

Proof of Concept

If users vote, their votes are transferred to the governance contract. So these voters cannot endorse any new proposals. To get the votes back, a new proposal should be activated. It can happen by either 1) activateProposal or 2) executeProposal. However, 1) activateProposal will revert if there is not enough endorsement 2) executeProposal will revert if the (for-vote - against-vote) is more than certain threshold

So if the both conditions cannot be met, no vote can be reclaimed and it will end up stuck there.

// Governance::activateProposal
// can update activeProposal
// but needs enough votes to activate a proposal
216         if (
217             (totalEndorsementsForProposal[proposalId_] * 100) <
218             VOTES.totalSupply() * ENDORSEMENT_THRESHOLD
219         ) {
220             revert NotEnoughEndorsementsToActivateProposal();
221         }

// Governance::vote
// if one votes, their votes will be transferred away
259         VOTES.transferFrom(msg.sender, address(this), userVotes);

// Governance::executeProposal
// can update activeProposal
// but can only execute if the voting condition is met
265     function executeProposal() external {
266         uint256 netVotes = yesVotesForProposal[activeProposal.proposalId] -
267             noVotesForProposal[activeProposal.proposalId];
268         if (netVotes * 100 < VOTES.totalSupply() * EXECUTION_THRESHOLD) {
269             revert NotEnoughVotesToExecute();
270         }

// Governance::reclaimVotes
// can only reclaim if the proposalId is not active anymore
302         if (proposalId_ == activeProposal.proposalId) {
303             revert CannotReclaimTokensForActiveVote();
304         }

When the Governance falls into the situation, the admin can resolve it by either 1) minting enough additional votes to make a new proposal active or 1) burning the votes of governance 2) propose, endorse and activate a proposal 3) mint the burned votes back to the governance 4) users can reclaim their votes as a new proposal is active

Tools Used

none

Recommended Mitigation Steps

Instead of transferring user's vote, snapshot can be used.

bahurum commented 2 years ago

Duplicate of #362, but this one is the best written imo.

fullyallocated commented 2 years ago

Duplicate of #376.

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