Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unspecified slippage allows sandwich attacks #2087

Open
codehawks-bot opened this issue Aug 8, 2023 · 0 comments
Open

Unspecified slippage allows sandwich attacks #2087

codehawks-bot opened this issue Aug 8, 2023 · 0 comments

Comments

@codehawks-bot
Copy link

Unspecified slippage allows sandwich attacks

Severity

High Risk

Relevant GitHub Links

https://github.com/Cyfrin/2023-07-beedle/blob/main/src/Fees.sol#L26-L44

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

Summary

The lack of a slippage value allows attackers to sandwich attack the transaction and extract the value.

Vulnerability Details

The swap in Fees.sol the sellProfits function executes a swap on Uniswap, but the amountOutMinimum value, which is accountable for slippage protection is at 0, which allows the swap to yield 0 tokens in return for the amount provided. This allows for MEVs to pick the transaction up from the mempool and to sandwich it by manipulating the pool, in which the swap is happening.

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 0 as a slippage amount makes this swap very vulnerable to sandwich attacks
                sqrtPriceLimitX96: 0
            });

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

Impact

The amount getting swapped will be completely lost.

Tools Used

Manual Review

Recommendations

Consider setting amountOutMinimum to some appropriate value, that includes a conservative amount of tolerance for price impact.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment