c4-bot-6 commented 5 months ago

Lines of code

https://github.com/code-423n4/2024-05-bakerfi/blob/main/contracts/core/strategies/StrategyLeverage.sol#L612 https://github.com/code-423n4/2024-05-bakerfi/blob/main/contracts/core/strategies/StrategyAAVEv3WSTETH.sol#L96 https://github.com/code-423n4/2024-05-bakerfi/blob/main/contracts/core/hooks/UseSwapper.sol#L72

Vulnerability details

Impact

User's funds are at risk because the swaps don't enforce a correct slippage to prevent MEV manipulation.

Proof of Concept

When withdrawing from a Vault, the Vault will burn shares from the withdrawer and will send him native ETH.

The withdrawal process looks like this:

The Vault computes the amount of ETH to withdraw for the specified amount of shares to be burnt
The Vault calls the StrategyLeverage.undeploy() function to proceed with the withdrawal of the corresponding amount of collateral from Aave.
The Strategy computes the delta of the Collateral and Debt expressed in their ETH value that is required to accommodate the withdrawal in Aave
The Strategy takes a flashloan in Balancer to borrow the deltaDebtInETH.
Using the flashloaned WETH, the strategy repays WETH debt in Aave
The Strategy withdraws (if enough deposits) the deltaCollateralInETH amount of collateral from Aave.
All the withdrawn collateral is swapped for WETH using a Uniswap.
The Strategy unwraps the difference between all the received WETH from the previous step and all the debt owed to balancer (flashloned amount + fees)
The native received from unwrapping WETH in the previous step is then sent to the Vault
Finally, the received native ETH from the Strategy is forwarded to the withdrawer.

The problem that allows MEV bots to steal from users is located in step 8.

When requesting the swap to the UniRouter to swap the withdrawn collateral for WETH, the Strategy doesn't compute a minimum amount of tokensOut to receive, it simply sets the amountOutMinimum parameter as 0.

Note, the same problem applies for the StrategyAAVEv3WSTETH._convertToWETH() function, but, since the base and derived functions does a swap of Collateral for WETH, I'm using the base function for this report.

> StrategyLeverage.sol

function _convertToWETH(uint256 amount) internal virtual returns (uint256) {
 // 1.Swap cbETH -> WETH/wstETH/rETH
 return
 //@audit-info => Swaps a collateral for WETH
 _swap(
 ISwapHandler.SwapParams(
 ierc20A(), // Asset In
 wETHA(), // Asset Out

 //@audit-info => swap of type EXACT_INPUT
 ISwapHandler.SwapType.EXACT_INPUT, // Swap Mode

 //@audit-info => Swapp all the withdrawn collateral
 amount, // Amount In

 //@audit-info => amountOut is set as 0
 0, // Amount Out

 _swapFeeTier, // Fee Pair Tier
 bytes("") // User Payload
 )
 );
}

> UseSwapper.sol
function _swap(
 ISwapHandler.SwapParams memory params
) internal override returns (uint256 amountOut) {
 ...

 // Exact Input
 if (params.mode == ISwapHandler.SwapType.EXACT_INPUT) {
 amountOut = _uniRouter.exactInputSingle(
 IV3SwapRouter.ExactInputSingleParams({
 tokenIn: params.underlyingIn,
 tokenOut: params.underlyingOut,
 amountIn: params.amountIn,

 //@audit-issue => Setting `amountOutMinimum` as 0. No slippage protection
 amountOutMinimum: 0,

 fee: fee,
 recipient: address(this),
 sqrtPriceLimitX96: 0
 })
 );
 ...
 } 
 ...
 ...
 ...

 ...

}

The problem is the type of swap used to swap Collateral for WETH is an EXACT_INPUT swap, which means, the UniRouter will swaps amountIn of one token for as much as possible of another token

Setting the amountOutMinimum as 0 lets the UniswapRouter swap all the amountIn for any amount of tokensOut, or in other words, the swap has no slippage protection and it basically will allow swapping all the withdrawn collateral from Aave for whatever amount of WETH. - The lack of slippage protection, plus the fact that sqrtPriceLimitX96 is set as 0 allows MEV bots to sandwich all depositor's withdrawals and manipulate the price of the Uniswap Pool where the swap will be made, causing the strategy to receive just enough WETH to repay balancer, but, stealing from the users all the extra WETH. - MEV bots will force the Strategy to swap at a manipulated rate where the Strategy will only receive the required WETH to pay the debt owed to balancer, leaving the withdrawer without any funds left.

For example, if 9 WETH were borrowed as a flashloan, and 9.5 wstETH were withdrawn from Aave. The Strategy only requires at the minimum to have 9 WETH + fees to repay the flashloan.

Let's suppose 1 wstETH is worth 1.2 ETH, that means, swapping 9.5wstETH could give 11.4 WETH, but, because the strategy doesn't enforce a minimum amountOut, a MEV bot can sandwich the withdrawal and get away with (11.4 WETH - 9 WETH) ===> 2.4 WETH (minus fees for the flashloan). The withdrawer would effectively receive close to 0 when withdrawing from the Vault because a mev bot was able to sandwich the tx and steal when swapping Collateral for WETH.

Tools Used

Manual Audit & UniswapRouter Documentation

Recommended Mitigation Steps

The recommendation to prevent this problem is to specify a value for the parameter amountOutMinimum, do not leave it as 0, and make sure to apply a reasonable slippage to the amountOutMinimum of WETH the Strategy wants to receive for swapping the withdrawn collateral from Aave, as a reference, the automatic slippage when swapping directly on the Uniswap app ranges from 0.1% to 5%

And, on the UseSwapper._swap() function, make sure to set the amountOutMinimum as the value passed on the params.amountOut

Assessed type

Uniswap

c4-judge commented 5 months ago

0xleastwood marked the issue as duplicate of #32