c4-bot-9 commented 9 months ago

Lines of code

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

Vulnerability details

Impact

The voting process of the DAO is vulnerable to flash loan attacks. It will comprise the governance process and make malicious proposals confirmed.

Proof of Concept

In the DAO's proposal voting, the weight of each voter's vote is determined by the number of SALT Tokens they have staked (i.e., uint256 userVotingPower = staking.userShareForPool( msg.sender, PoolUtils.STAKED_SALT );) https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/dao/Proposals.sol#L259

// Cast a vote on an open ballot
    function castVote( uint256 ballotID, Vote vote ) external nonReentrant
        {
        Ballot memory ballot = ballots[ballotID];

        // Require that the ballot is actually live
        require( ballot.ballotIsLive, "The specified ballot is not open for voting" );

        // Make sure that the vote type is valid for the given ballot
        if ( ballot.ballotType == BallotType.PARAMETER )
            require( (vote == Vote.INCREASE) || (vote == Vote.DECREASE) || (vote == Vote.NO_CHANGE), "Invalid VoteType for Parameter Ballot" );
        else // If a Ballot is not a Parameter Ballot, it is an Approval ballot
            require( (vote == Vote.YES) || (vote == Vote.NO), "Invalid VoteType for Approval Ballot" );

        // Make sure that the user has voting power before proceeding.
        // Voting power is equal to their userShare of STAKED_SALT.
        // If the user changes their stake after voting they will have to recast their vote.

        uint256 userVotingPower = staking.userShareForPool( msg.sender, PoolUtils.STAKED_SALT );
        require( userVotingPower > 0, "Staked SALT required to vote" );

        // Remove any previous votes made by the user on the ballot
        UserVote memory lastVote = _lastUserVoteForBallot[ballotID][msg.sender];

        // Undo the last vote?
        if ( lastVote.votingPower > 0 )
            _votesCastForBallot[ballotID][lastVote.vote] -= lastVote.votingPower;

        // Update the votes cast for the ballot with the user's current voting power
        _votesCastForBallot[ballotID][vote] += userVotingPower;

        // Remember how the user voted in case they change their vote later
        _lastUserVoteForBallot[ballotID][msg.sender] = UserVote( vote, userVotingPower );

        emit VoteCast(msg.sender, ballotID, vote, userVotingPower);
        }

Once the weight exceeds the quorum, anyone can finalize the vote by executing the finalizeBallot() function （https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/dao/DAO.sol#L278）.

// Finalize the vote on a specific ballot.
    // Can be called by anyone, but only actually finalizes the ballot if it can be finalized.
    function finalizeBallot( uint256 ballotID ) external nonReentrant
        {
        // Checks that ballot is live, and minimumEndTime and quorum have both been reached
        require( proposals.canFinalizeBallot(ballotID), "The ballot is not yet able to be finalized" );

        Ballot memory ballot = proposals.ballotForID(ballotID);

        if ( ballot.ballotType == BallotType.PARAMETER )
            _finalizeParameterBallot(ballotID);
        else if ( ballot.ballotType == BallotType.WHITELIST_TOKEN )
            _finalizeTokenWhitelisting(ballotID);
        else
            _finalizeApprovalBallot(ballotID);
        }

However, this voting mechanism could be vulnerable to a flash loan attack.

In such an attack, someone could borrow a sufficient number of SALT Tokens through a flash loan, call the castVote function to stake and vote with these borrowed tokens, and then, within the same transaction, call the finalizeBallot function to conclude the voting on any proposal.

Considering that DAO proposals often involve critical modifications to system parameters, such flash loan attacks are extremely dangerous.

Tools Used

Manual Analysis

Recommended Mitigation Steps

Add protection schemes against flashloan attacks. For example, enforce a rule that prohibits the executing the castvote() and the finalizeBallot()functions within the same transaction or block.

Assessed type

Governance