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

5 stars 4 forks source link

Governance: the user cannot vote anymore if already voted, even if the user has additional votes #413

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

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

Vulnerability details

Impact

The votes do not reflect the votes the user have

Proof of Concept

In the line 247 of the below snippet, the vote function checks whether the user already voted to the current proposal. This prevents the users from voting additionally.

If the user forgot to reclaim from the past votes, or the user got minted additionally after the vote, the user cannot use those votes for the current proposal anymore. This is unfair, especially because the user cannot bypass this by transferring the votes, as transfer is disabled.

// Governance::vote
240     function vote(bool for_) external {
241         uint256 userVotes = VOTES.balanceOf(msg.sender);
242
243         if (activeProposal.proposalId == 0) {
244             revert NoActiveProposalDetected();
245         }
246
247         if (userVotesForProposal[activeProposal.proposalId][msg.sender] > 0) {
248             revert UserAlreadyVoted();
249         }
250
251         if (for_) {
252             yesVotesForProposal[activeProposal.proposalId] += userVotes;
253         } else {
254             noVotesForProposal[activeProposal.proposalId] += userVotes;
255         }
256
257         userVotesForProposal[activeProposal.proposalId][msg.sender] = userVotes;
258
259         VOTES.transferFrom(msg.sender, address(this), userVotes);
260
261         emit WalletVoted(activeProposal.proposalId, msg.sender, for_, userVotes);
262     }

Tools Used

none

Recommended Mitigation Steps

Allow to vote additionally.

fullyallocated commented 2 years ago

Duplicate of #275