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

11 stars 6 forks source link

No arrangement for when exchange does not start on first voting attempt. #860

Closed c4-bot-3 closed 9 months ago

c4-bot-3 commented 9 months ago

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/launch/BootstrapBallot.sol#L71 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/launch/BootstrapBallot.sol#L84

Vulnerability details

Impact

The BootstrapBallot.sol contract allows DAO to vote and start exchange. When startExchangeYes > startExchangeNo, calls are made to exchangeConfig.initialDistribution().distributionApproved() and exchangeConfig.dao().pools().startExchangeApproved() to distribute SALT and start exchange. Also, all voters are allowed to claim airdrop.

The problem here is that protocol failed to consider when the first poll does not start the exchange, this means that the exchange will remain closed if the first poll favors startExchangeNo

The purpose of the ballot is to ensure that most DAO members are in support of exchange launch before it is launched. However, majority might vote against it for various reasons like; technical glitches, low marketing, perceived unreadiness, this does not mean that they have intention to keep the exchange closed forever.

However, after the first ballot, no other ballot can be finalized due to https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/launch/BootstrapBallot.sol#L84 and https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/launch/BootstrapBallot.sol#L71

Another implication is that funds will remain in InitialDistribution contract if startExchangeYes < startExchangeNo and will not be distributed as designed until startExchangeYes > startExchangeNo

Proof of Concept

Let us consider an instance where after the deployment of BootstrapBallot.sol, most DAO members voted against the launch of exchange due to the fact that they believe more marketing should be done, so launch should be postponed for a while.

When it is time for a revote to occur to launch the exchange the BootstrapBallot.sol contract will no longer finalize any other ballot.

    function finalizeBallot() external nonReentrant
        {
        require( ! ballotFinalized, "Ballot has already been finalized" );
        require( block.timestamp >= completionTimestamp, "Ballot is not yet complete" );

        if ( startExchangeYes > startExchangeNo )
            {
            exchangeConfig.initialDistribution().distributionApproved();
            exchangeConfig.dao().pools().startExchangeApproved();

            startExchangeApproved = true;
            }

        emit BallotFinalized(startExchangeApproved);

        ballotFinalized = true;
        }

Tools Used

Manual review.

Recommended Mitigation Steps

Add flexibility to the BootstrapBallot.sol contract which will enable the start and finalization of another ballot if the exchange is yet to start(startExchangeApproved = false). Only time when it should be impossible to finalize a ballot is when startExchangeApproved = true;

Assessed type

Context

c4-judge commented 9 months ago

Picodes marked the issue as duplicate of #606

c4-judge commented 8 months ago

Picodes changed the severity to QA (Quality Assurance)