Arcade.xyz - 0xWaitress's results

Lines of code

https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeTreasury.sol#L384-L394

Vulnerability details

Impact

gscApprove can overwrite the approved allowance of the same spender from previously approved value

gscApprove, approveSmallSpend, approveMediumSpend, approveLargeSpend all calls the same underlying _approve.

Let's say the DAO approved a MediumSpend budget on spender A. Coincidentally, gscWhitelist member would also like to approve some allowance to spender A on the same token. gscApprove would then over-write the previous allowance.

Alternatively, gscApprove caller can also maliciously over-write the allowance of the previously set value for a spender by DAO; or allowance set by other gsc members, by backrunning with a smaller allowance(1wei for example). This causes damage to the daily operation of DAO treasury.

    function gscApprove(
        address token,
        address spender,
        uint256 amount
    ) external onlyRole(GSC_CORE_VOTING_ROLE) nonReentrant {
        if (spender == address(0)) revert T_ZeroAddress("spender");
        if (amount == 0) revert T_ZeroAmount();

        // Will underflow if amount is greater than remaining allowance
        gscAllowance[token] -= amount;

        _approve(token, spender, amount, spendThresholds[token].small);
    }

    function _approve(address token, address spender, uint256 amount, uint256 limit) internal {
        // check that after processing this we will not have spent more than the block limit
        uint256 spentThisBlock = blockExpenditure[block.number];
        if (amount + spentThisBlock > limit) revert T_BlockSpendLimit();
        blockExpenditure[block.number] = amount + spentThisBlock;

        // approve tokens
        IERC20(token).approve(spender, amount);

        emit TreasuryApproval(token, spender, amount);
    }

https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeTreasury.sol#L384-L394 https://github.com/code-423n4/2023-07-arcade/blob/main/contracts/ArcadeTreasury.sol#L189-L201

Proof of Concept

Tools Used

Recommended Mitigation Steps

Consider using increaseAllowance instead.

Assessed type

Context