Sturdy (Aura Integration And Leverage)

The functions `withdrawWithFlashloan()`, `zapDeposit()`, and `zapLeverageWithFlashloan()` within the `GeneralLevSwap` contract have ineffective slippage mechanisms and permit swaps to be made at very undesirable rates. This leaves users vulnerable to sandwich attacks and front-running. In the worst scenario, it could result in users losing far more collateral tokens than expected when repaying a loan through `GeneralLevSwap`. The slippage control of each function is ineffective in a different way. ### 1. `withdrawWithFlashloan()` While the function does accept a `_slippage` parameter, the only time this parameter is used is when withdrawing collateral from the vault contract: `IGeneralVault(VAULT).withdrawCollateral(COLLATERAL, _amount, _slippage, address(this));`. This will ensure that the amount withdrawn from the vault is within the `_slippage` tolerance of `_amount`. However, this does nothing to protect against severe slippage when swapping from the collateral asset to the borrowing asset and vice versa. In particular, severe slippage may occur in the following two swaps: * `L328:_swapFrom(_borrowingAsset);` * This call swaps all the user's withdrawn collateral asset tokens for borrowing asset tokens. The amount of collateral withdrawn on behalf of the user is the maximum amount a user can withdraw while remaining in a healthy (non-liquidatable position). * If the entire debt was repaid on `L292:_repay(_borrowingAsset, _borrowedAmount, _user);`, then all the collateral tokens of the user are swapped. If the slippage is severe enough that the swap only produces enough borrowing asset tokens to repay the flash loan and swap back to the `_requiredAmount`, the user will lose all their collateral. * In the current implementation of `AURAWSTETHWETHLevSwap`, we note that the `_swapFrom()` function has a hardcoded slippage of `10%`. While this limits the impact to some extent, 10% is a severe amount of slippage, and the loss suffered by the user could be much greater than `10%` as seen in the exploit scenario. * `L253:_swapTo(_borrowingAsset, IERC20(_borrowingAsset).balanceOf(**address**(this)));` * This call swaps whatever borrowing asset is left after repaying the flash loan into collateral asset tokens. If `_swapFrom()` suffered from severe slippage, then this swap may have negative slippage. However, the damage is already done, as seen in the exploit scenario section. * Unlike `_swapFrom()`, the function does not even have the hardcoded slippage control of `10%` within the `AURAWSTETHWETHLevSwap` contract. ### 2. `zapDeposit()` * The `zapDeposit()` function swaps the borrowable asset for the collateral asset. However, no slippage parameters are available for the function, meaning that any amount of slippage is possible. This could result in the user's zapped `_principal` losing most of its value. ### 3. `zapLeverageWithFlashloan()` * The `zapLeverageWithFlashloan()` function does have a `_slippage` parameter. However, this parameter has no impact on the first swap it makes: `L444:uint256 collateralAmount = _swapTo(_zappingAsset, _principal);`. Thus the function suffers from the same problem as `zapDeposit()` above.

The `BALWSTETHWETHOracle.sol` contract retrieves the price of `STETH` by querying a Chainlink price feed: ` (, int256 wstETHPrice, , , ) = WSTETH.latestRoundData();`. However, several unchecked assumptions are made about the price, including its staleness, validity, and the number of decimals it has. The consequences of an incorrect price could be severe for the protocol, including the loss of user funds. The following should be done to ensure a valid price: 1. Validate that the `updatedAt` value returned by `WSTETH.latestRoundData()` indicates a timestamp that is not too old. 2. Validate that the price returned by `WSTETH.latestRoundData()` is greater than zero. 3. Rather than assuming that the price has `18` decimals, explicitly check the number of decimals from the price feed through the `decimals()` function.

The function signatures from `_swapFrom()` and `_swapTo()` seem to suggest that `_amount` of `_borrowingAsset` is swapped to another asset or being swapped for, respectively. However, in the derived implementation in the `AURAWSTETHWETHLevSwap` contract, the `_borrowingAsset` parameter remains unused in both methods. The `swapTo()` function always swaps WETH to Balancer LP Token by adding them to the pool and the `swapTo()` function swaps the Balancer LP Token to WETH by exiting the pool. The same issue can be spotted in the other contract derived from the `GeneralLevSwap` contract, namely the `ETHSTETHLevSwap` contract. Also, the implementation of the function `getAvailableStableCoins()` in the `AURAWSTETHWETHLevSwap` contract returns the WETH-address, which of course is not a stablecoin.