code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/policies/Governance.sol#L93 https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/policies/Governance.sol#L302 https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/policies/Governance.sol#L288 https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/policies/Governance.sol#L231 https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/Kernel.sol#L239 https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/Kernel.sol#L270 https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/policies/Governance.sol#L302

Vulnerability details

Impact

Olympus votes can be locked in Governance.sol OlympusGovernance contract.

Proof of Concept

When a user votes for a proposal, their current balance of VOTE is transferred to the OlympusGovernance contract. Those votes can be reclaimed once the proposal is no more the current active proposal. https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/policies/Governance.sol#L302

If the active proposal cannot be executed (reverts) due to the instructions it can result in a scenario where the active proposal cannot be changed even if the grace period has finished.

There are only two ways to change the activeProposal, executing it or activating another proposal when the GRACE_PERIOD is finished.

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

The way to do it via activateProposal() is by replacing the activeProposal, but the new proposal needs at least ENDORSEMENT_THRESHOLD of the votes (currently 20% of the total supply) The problem is that if the majority (more than 80% of the total supply) of the votes are locked in the current activeProposal and cannot be reclaimed they can't be used to endorse a new proposal and the activeProposal will not change and votes will remain locked.

A possible instruction that cannot be executed is installing a module already installed, but many others are possible and it is not obvious to the voters that the instructions are going to revert when executed. The contract should verify these particular cases, not the voters.

https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/Kernel.sol#L239 https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/Kernel.sol#L270

Tools Used

Recommended Mitigation Steps

A possible way to fix the problem is by letting the user claim their votes if the grace period of the active proposal is finished. Change : if (proposalId_ == activeProposal.proposalId) { revert CannotReclaimTokensForActiveVote(); }

By " if(proposalId_ == activeProposal.proposalId && block.timestamp < activeProposal.activationTimestamp + GRACE_PERIOD){ revert CannotReclaimTokensForActiveVote(); }

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

fullyallocated commented 2 years ago

This issue is brought up before but without the existing tokens locked affecting endorsements. Good catch.

0xean commented 2 years ago

closing of dupe of #376

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