c4-bot-10 commented 9 months ago

Lines of code

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

Vulnerability details

Impact

sendSALT may send more than 5% of the tokens.

Proof of Concept

    function proposeSendSALT( address wallet, uint256 amount, string calldata description ) external nonReentrant returns (uint256 ballotID)
        {
        require( wallet != address(0), "Cannot send SALT to address(0)" );

        // Limit to 5% of current balance
@>      uint256 balance = exchangeConfig.salt().balanceOf( address(exchangeConfig.dao()) );
        uint256 maxSendable = balance * 5 / 100;
@>      require( amount <= maxSendable, "Cannot send more than 5% of the DAO SALT balance" );

        // This ballotName is not unique for the receiving wallet and enforces the restriction of one sendSALT ballot at a time.
        // If more receivers are necessary at once, a splitter can be used.
        string memory ballotName = "sendSALT";
        return _possiblyCreateProposal( ballotName, BallotType.SEND_SALT, wallet, amount, "", description );
        }

When proposeSendSALT, the amount cannot exceed 5% of the number of tokens held in the dao contract, The problem is that the balance of tokens in the contract is fluid. After propose is created, it needs to be voted, and the transfer transaction will be executed. During this time, the number of tokens in the dao contract is likely to change. If the number of tokens is smaller, the percentage of tokens actually sent will be larger than when the propose was created, so more than 5% of tokens may be sent.

Tools Used

vscode, manual

Recommended Mitigation Steps

When setting the maximum value, add a parameter for the allowable range of variation.

Assessed type

Other

c4-judge commented 9 months ago

Picodes changed the severity to QA (Quality Assurance)