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

11 stars 6 forks source link

`requiredQuorum` may be lower than the actual value #689

Closed c4-bot-8 closed 9 months ago

c4-bot-8 commented 9 months ago

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/dao/Proposals.sol#L334

Vulnerability details

Impact

requiredQuorum may be lower than the actual value, it is possible that the vote will pass early.

Proof of Concept

A canFinalizeBallot is used to determine whether a quorum is present.

    function canFinalizeBallot( uint256 ballotID ) external view returns (bool)
        {
        Ballot memory ballot = ballots[ballotID];
        if ( ! ballot.ballotIsLive )
            return false;

        // Check that the minimum duration has passed
        if (block.timestamp < ballot.ballotMinimumEndTime )
            return false;

        // Check that the required quorum has been reached
        if ( totalVotesCastForBallot(ballotID) < requiredQuorumForBallotType( ballot.ballotType ))
            return false;

        return true;
        }

requiredQuorumForBallotType through salt token to calculate the total supply minimumQuorum:

    function requiredQuorumForBallotType( BallotType ballotType ) public view returns (uint256 requiredQuorum)
        {
        ....
        uint256 totalSupply = ERC20(address(exchangeConfig.salt())).totalSupply();
        uint256 minimumQuorum = totalSupply * 5 / 1000;

        if ( requiredQuorum < minimumQuorum )
            requiredQuorum = minimumQuorum;
        }

The problem is that totalSupply is not a fixed value, and minimumQuorum is lower than the actual value when totalSupply decreases:

    function burnTokensInContract() external
    {
        uint256 balance = balanceOf( address(this) );
        _burn( address(this), balance );

        emit SALTBurned(balance);
    }

Tools Used

vscode, manual

Recommended Mitigation Steps

Use the snapshot feature when voting, such as using openzeppelin's governance library.

Assessed type

Governance

c4-judge commented 9 months ago

Picodes marked the issue as duplicate of #46

c4-judge commented 8 months ago

Picodes marked the issue as satisfactory