Cyfrin / 2023-07-beedle

21 stars 20 forks source link

Fees.sol#L30-L40 : Lack of slippage protection for uniswap swapping. #2049

Open codehawks-bot opened 1 year ago

codehawks-bot commented 1 year ago

Fees.sol#L30-L40 : Lack of slippage protection for uniswap swapping.

Severity

High Risk

Relevant GitHub Links

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

Summary

Fees.sol has the sellProfits function which will take the contracts balance and tries to swap and get the amount and then transfer to the staking contract.

the issue here is, there is no protection from MEV bots and other price manipulation attack which will lead to loss of funds. This is because there is not protection from slippage issue.

Vulnerability Details

    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, -------------->>> audit find . no min value is specified.
                sqrtPriceLimitX96: 0
            });

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

Impact

Loss of funds due to MEV bots attack or price manipulation attacks.

Tools Used

Manual review.

Recommendations

Include the slippage value this would ensure that the loss would not go beyond this specified limit.

PatrickAlphaC commented 1 year ago

I don't like the recommendation. I think a better option would be to use a auction.