Sandwich attack to steal all ERC-20 tokens in the Fees contract

Severity

High Risk

Relevant GitHub Links

https://github.com/Cyfrin/2023-07-beedle/blob/658e046bda8b010a5b82d2d85e824f3823602d27/src/Fees.sol#L38-L39

Summary

The Fees::sellProfits() lacks slippage protection, resulting in being attacked by a sandwich attack to drain all locked ERC-20 tokens.

Vulnerability Details

The sellProfits() is a permissionless function that can be called by anyone. The function lacks slippage protection (the parameters amountOutMinimum and sqrtPriceLimitX96 are set to 0) when swapping tokens through Uniswap's pools.

In this way, an attacker can launch a sandwich attack with a flash loan to drain all ERC-20 tokens (e.g., USDC, DAI, CRV, etc.) locked in the Fees contract.

For instance, to drain all USDC, consider the following proof-of-concept.

Attacker borrows a flash loan for USDC and buys WETH from Uniswap's WETH/USDC pool.
Attacker executes the sellProfits(USDC).
The sellProfits() will spend all locked USDC for buying WETH at a very high price.
Attacker sells the previously obtained WETH for USDC at the same pool and repays the flash loan.
Attacker takes all locked USDC as profit.

Moreover, an attacker can perform steps 1-5 above to steal other ERC-20 tokens locked in the Fees contract.

    function sellProfits(address _profits) public {
        require(_profits != WETH, "not allowed");
        uint256 amount = IERC20(_profits).balanceOf(address(this));

        ISwapRouter.ExactInputSingleParams memory params = ISwapRouter
            .ExactInputSingleParams({
                tokenIn: _profits,
                tokenOut: WETH,
                fee: 3000,
                recipient: address(this),
                deadline: block.timestamp,
                amountIn: amount,
@>              amountOutMinimum: 0,
@>              sqrtPriceLimitX96: 0
            });

        amount = swapRouter.exactInputSingle(params);
        IERC20(WETH).transfer(staking, IERC20(WETH).balanceOf(address(this)));
    }

https://github.com/Cyfrin/2023-07-beedle/blob/658e046bda8b010a5b82d2d85e824f3823602d27/src/Fees.sol#L38-L39

Impact

An attacker can drain all ERC-20 tokens (e.g., USDC, DAI, CRV, etc.) locked in the Fees contract. Therefore, I consider this vulnerability a high-risk issue.

Tools Used

Manual Review

Recommendations

I recommend adding the onlyOwner modifier and setting the amountOutMinimum parameter to protect price slippage from MEV bots. If necessary, specify the sqrtPriceLimitX96 parameter to set a stop price.

Cyfrin / 2023-07-beedle