Total salt supply permanently locked if launch vote outcome is no or a draw

[c4-bot-3]

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/launch/InitialDistribution.sol#L50 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/launch/BootstrapBallot.sol#L69 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/launch/BootstrapBallot.sol#L48 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/dev/Deployment.sol#L237

Vulnerability details

Summary:

The protocol features a decentralized launch process, where whitelisted users vote to initiate the exchange. The voting period is governed by ballotDuration. Once this period concludes, any user can call the finalizeBallot function to finalize the vote. An issue with the current implementation is that if the voting results in more 'no' than 'yes' votes or a draw, the entire SALT supply in the initialDistribution contract becomes permanently locked.

Vulnerability Details:

The finalizeBallot function, which can be invoked by anyone post-ballot duration, concludes the voting process for starting the exchange and triggers the distribution of SALT tokens if the voting results in more 'yes' than 'no'. Regardless of the voting outcome, this function sets ballotFinalized to true. This is a problem because if the majority vote is 'no', the function cannot be re-invoked due to ballotFinalized being set to true.

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;
    }

The InitialDistribution contract will receive the entire SALT supply (100 million tokens) before finalizing the ballot. This setup is necessary because the distributionApproved function is designed to operate only when the contract holds the exact amount of 100 million SALT tokens

• This is also the order used in the Deployment contract.

Furthermore the distributionApproved function is exclusively callable by the bootstrapBallot contract., with the bootstrapBallot address unable to be changed.

function distributionApproved() external {
        require(
            msg.sender == address(bootstrapBallot),
            "InitialDistribution.distributionApproved can only be called from the BootstrapBallot contract"
        );
        ...
    }

Impact:

In the event of a 'no' majority in the ballot, the entire SALT supply remains locked in the InitialDistribution contract, rendering the protocol dysfunctional. This scenario cannot be rectified, nor can the funds be redirected, as the only transfer mechanism is permanently disabled.

Proof Of Concept

function test_finalizeBallotFailedVoteFundsLocked() public {
    // Voting stage (yesVotes: 2, noVotes: 0)
    bytes memory sig = abi.encodePacked(aliceVotingSignature);
    vm.startPrank(alice);
    bootstrapBallot.vote(false, sig);
    vm.stopPrank();

    sig = abi.encodePacked(bobVotingSignature);
    vm.startPrank(bob);
    bootstrapBallot.vote(false, sig);
    vm.stopPrank();

    // Increase current blocktime to be greater than completionTimestamp
    vm.warp( bootstrapBallot.completionTimestamp());

    assertEq( salt.balanceOf(address(initialDistribution)), 100000000 ether);

    // Call finalizeBallot()
    bootstrapBallot.finalizeBallot();

    // Verify that the salt total is still in the initialDistribution contract
    assertEq( salt.balanceOf(address(initialDistribution)), 100000000 ether);

    // calling finalizeBallot again should fail
    vm.expectRevert("Ballot has already been finalized");
    bootstrapBallot.finalizeBallot();
    }

Tools Used:

• Manual analysis
• Foundry

Recommendation:

The protocol should implement a mechanism allowing for a re-vote under different parameters or provide an alternative fund transfer option in the event of a 'no' majority or draw. This will prevent the total SALT supply from being permanently locked.

Assessed type

DoS

[c4-judge]

Picodes marked the issue as primary issue

[c4-sponsor]

othernet-global (sponsor) disputed

[othernet-global]

Yes, this is correct. If the BootstrapBallot is not successful then the protocol should not start. This is by design. The contracts would have to be redeployed and be voted on again.

[Picodes]

Downgrading to Low as this only leads to a redeployment

[c4-judge]

Picodes changed the severity to QA (Quality Assurance)