tokenWhitelistingBallotWithTheMostVotes might return an incorrect value

[c4-bot-5]

Lines of code

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

Vulnerability details

Impact

DAO can propose whitelisting of a token by calling proposeTokenWhitelisting(), with more than one poolsConfig.numberOfWhitelistedPools(), only the token with the highest support vote can be whitelisted at a certain period.

// Fail to whitelist for now if this isn't the whitelisting proposal with the most votes - can try again later.
uint256 bestWhitelistingBallotID = proposals.tokenWhitelistingBallotWithTheMostVotes();
require( bestWhitelistingBallotID == ballotID, "Only the token whitelisting ballot with the most votes can be finalized" );

However, the problem here is that proposals.tokenWhitelistingBallotWithTheMostVotes(); might return an incorrect value under certain condition. For instance when more than one token has the same number of Yes as the highest value of yes.

Proof of Concept

Let us consider a case study;

1. Five tokens were proposed for whitelisting

2. After voting is done; Quorum = 1,000 Token A(BNB) has 600 YES and 400 N0 Token B(PEP) has 300 YES and 700 N0 Token C(AVAX) has 600 YES and 400 N0 Token D(WIN) has 300 YES and 700 N0 Token E(SHB) has 590 YES and 410 NO We can see here that more than one token has the highest number of YES, but when proposals.tokenWhitelistingBallotWithTheMostVotes(); is called, Token A will be returned as the one with the most YES vote, which is not correct in this case because Token C also has 600 YES votes. However, Token A will be whitelisted because of its placement in the array and not because it has the highest YES votes while Token C with the same number of YES votes will be discarded.

   
       {
       uint256 quorum = requiredQuorumForBallotType( BallotType.WHITELIST_TOKEN);
   uint256 bestID = 0;
   uint256 mostYes = 0;
   for( uint256 i = 0; i < _openBallotsForTokenWhitelisting.length(); i++ )
   {
   uint256 ballotID = _openBallotsForTokenWhitelisting.at(i);
   uint256 yesTotal = _votesCastForBallot[ballotID][Vote.YES];
   uint256 noTotal = _votesCastForBallot[ballotID][Vote.NO];
   
   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;
   }
   }

return bestID; }

## Tools Used
Manual review.

## Recommended Mitigation Steps
Modify the code in such a way that, when there is more than one token with the highest number of YES, have a condition to settle for that. 
This might be to discard the ballot or to find the token with the highest number of total votes(highest participation).

## Assessed type

Invalid Validation

[c4-judge]

Picodes changed the severity to QA (Quality Assurance)

[Picodes]

Downgrading to QA as this is an arbitrary edge case.

[c4-judge]

Picodes marked the issue as grade-b