Lines of code

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

Vulnerability details

Impact

Ballot with highest yes vote won't be selected for token whitelisting under certain circumstances. Ballot which are created early are given preference which might not be fair

Proof of Concept

Lets say 3 ballots exists for token whitelisting
Below are the param used

quorum=100

Ballot 1:
yesVotes=80
noVotes=30

Ballot 2:
yesVotes=80
noVotes=20

Ballot 3:
yesVotes=70
noVotes=30

Now once Ballots are finalized for whitelisting, below call sequence occurs:

_finalizeTokenWhitelisting -> proposals.tokenWhitelistingBallotWithTheMostVotes();

tokenWhitelistingBallotWithTheMostVotes will iterate through each ballot and find the one with most yesVotes like below:

if ( (yesTotal + noTotal) >= quorum ) // Make sure that quorum has been reached
            if ( yesTotal > noTotal )  // Make sure the token vote is favorable
            if ( yesTotal > mostYes )  // Make sure these are the most yes votes seen
                {
                bestID = ballotID;
                mostYes = yesTotal;
                }

Below is the run sequence:

Ballot 1:

(yesTotal + noTotal) >= quorum is true since 80+30>=100
yesTotal > noTotal is true since 80>30
mostYes=80
bestID=Ballot 1

Ballot 2:

(yesTotal + noTotal) >= quorum is true since 80+20>=100
yesTotal > noTotal is true since 80>20
mostYes=80
yesTotal > mostYes is false since 80>80 is false
bestID remains Ballot 1

As we can see that even when both Ballot have equal yesVotes, Ballot1 gets chosen, meaning Ballot1 asset gets whitelisted which is unfair for Ballot2 voters

uint256 bestWhitelistingBallotID = proposals.tokenWhitelistingBallotWithTheMostVotes();
            require( bestWhitelistingBallotID == ballotID, "Only the token whitelisting ballot with the most votes can be finalized" );

            // All tokens are paired with both WBTC and WETH, so whitelist both pairings
            poolsConfig.whitelistPool( pools,  IERC20(ballot.address1), exchangeConfig.wbtc() );
            poolsConfig.whitelistPool( pools,  IERC20(ballot.address1), exchangeConfig.weth() );

            bytes32 pool1 = PoolUtils._poolID( IERC20(ballot.address1), exchangeConfig.wbtc() );
            bytes32 pool2 = PoolUtils._poolID( IERC20(ballot.address1), exchangeConfig.weth() );

Recommended Mitigation Steps

Consider a strategy in case yes votes are equal in multiple Ballots. Probably a random function could be created which selected one ballot from list of tie ballots

Assessed type

Governance

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