c4-bot-7 commented 8 months ago

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/main/src/launch/BootstrapBallot.sol#L69-L85 https://github.com/code-423n4/2024-01-salty/blob/main/src/launch/Airdrop.sol#L56-L85 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/launch/InitialDistribution.sol#L50-L74 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/dao/DAO.sol#L155-L228

Vulnerability details

Impact

Detailed description of the impact of this finding. The codebase indicates that the users will decide whether exchange will start up or not by calling vote function during the voting period completionTimestamp which is initialized in constructor.In case malicous actors manipulate the result of bootstrap ballot and veto the launch of exchange,the airdrop won't never be distributed because in order to call claimAirdrop() function,allowClaiming() function should be called before however this function can be called only by address(exchangeConfig.initialDistribution() as the require block inside allowClaiming function indicates but once the bootstrap ballot is finished if the majority rejected starting up the exchange than there won't be another voting period for this purpose and the airdrops will be unclaimable until forever

Proof of Concept

Inside finalizeBallot function once ballot finalized if statement indicates that in case more people accepted starting exchange than distribution approved function of initialDistribution contract will be called and exchange will be approved.

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

However if the majority of the voters veto this proposal than the governance mechanism indicates that there is no way to claim airdrops since claimAirdrop function checks if claiming allowed and it cannot be allowed without calling allowClaiming() function and this function can be called only by address(exchangeConfig.initialDistribution()).

 function allowClaiming() external
        {
        //this function can be called by only exchangeConfig's initialDistribution() method
        require( msg.sender == address(exchangeConfig.initialDistribution()), "Airdrop.allowClaiming can only be called by the InitialDistribution contract" );
        require( ! claimingAllowed, "Claiming is already allowed" );
        require(numberAuthorized() > 0, "No addresses authorized to claim airdrop.");

        // All users receive an equal share of the airdrop.
        uint256 saltBalance = salt.balanceOf(address(this));
        saltAmountForEachUser = saltBalance / numberAuthorized();

        // Have the Airdrop approve max so that that xSALT (staked SALT) can later be transferred to airdrop recipients.
        salt.approve( address(staking), saltBalance );

        claimingAllowed = true;
        }

    function claimAirdrop() external nonReentrant
        {
        //condition below reverts if claiming is not allowed
        require( claimingAllowed, "Claiming is not allowed yet" );
        require( isAuthorized(msg.sender), "Wallet is not authorized for airdrop" );
        require( ! claimed[msg.sender], "Wallet already claimed the airdrop" );

        // Have the Airdrop contract stake a specified amount of SALT and then transfer it to the user
        staking.stakeSALT( saltAmountForEachUser );
        staking.transferStakedSaltFromAirdropToUser( msg.sender, saltAmountForEachUser );

        claimed[msg.sender] = true;
        }

Tools Used

Manual review

Recommended Mitigation Steps

In order to fix this issue there are two different approaches. 1.Adding a function to launch the bootstrap ballot again whenever it is needed and ensure that it can be called by only owner address. 2.Seperating airdrops and bootstrap ballot by changing governant mechanism or by simply adding an owner address to the Airdrop contract and changing require( msg.sender == address(exchangeConfig.initialDistribution()), "Airdrop.allowClaiming can only be called by the InitialDistribution contract" ); check of allowClaiming function to following condition : require( msg.sender == owner, "Airdrop.allowClaiming can only be called by the owner" );

Assessed type

Governance

c4-judge commented 8 months ago

Picodes marked the issue as duplicate of #606

c4-judge commented 7 months ago

Picodes changed the severity to QA (Quality Assurance)