approved ballots might be finalized in reversed order

[c4-bot-10]

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/dao/Proposals.sol#L385-L400 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/dao/DAO.sol#L278-L291

Vulnerability details

Impact

While DAO.finalizeBallot is called to finalized a ballot, it just check if the ballot meet some criterion, it doesn't contain any information about predecessor or dependency. In such case, two ballots might be finalized in reversed order to achieve different goals.

Proof of Concept

In Proposals.canFinalizeBallot, the funciton only checks if ballot.ballotIsLive, the minimum duration, and required quorum has been reached

384         // Checks that ballot is live, and minimumEndTime and quorum have both been reached.
385         function canFinalizeBallot( uint256 ballotID ) external view returns (bool)
386                 {
387         Ballot memory ballot = ballots[ballotID];
388         if ( ! ballot.ballotIsLive )
389                 return false;
390
391         // Check that the minimum duration has passed
392         if (block.timestamp < ballot.ballotMinimumEndTime )
393             return false;
394
395         // Check that the required quorum has been reached
396         if ( totalVotesCastForBallot(ballotID) < requiredQuorumForBallotType( ballot.ballotType ))
397             return false;
398
399         return true;
400             }

Supposed in a case for configuration cfg_X updating, the range for cfg_X is 0<=cfg_X<=5, and current cfg_X is 5, ballot_m is proposed to decrease cfg_X to 4, and after sometime, there is another ballot_n to increase cfg_X to 5 again. Both of the proposal have met the requirement in function canFinalizeBallot, but haven't been finalized yet:

1. if ballot_m is executed first, and ballot_n is executed later, the cfg_X will be 5.
2. but ballot_n is executed first, and ballot_m is executed later, the cfg_X will be 4.

Tools Used

VS

Recommended Mitigation Steps

Assessed type

Other

[c4-judge]

Picodes marked the issue as primary issue

[c4-sponsor]

othernet-global (sponsor) disputed

[othernet-global]

Executing approved ballots in arbitrary order is acceptable.

[c4-judge]

Picodes changed the severity to QA (Quality Assurance)

[Picodes]

Downgrading to Low as although this could be a pain in practice and increase delays to execute complex operations this is of Low severity to me