Sturdy security record

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.

Unlike AAVE, the Sturdy protocol draws a line between collateral and debt assets. A collateral asset can never be a debt asset and vice versa. However, additional care must be taken to ensure that certain actions can only be performed on reserves of a particular type (debt or collateral). We have listed the cases where additional checks must be added to ensure that each reserve type is acted on appropriately: * `LendingPool` * `depositYield()`: Validate that `asset` is not a collateral asset. * `getYield()`: Validate that `asset` is a collateral asset. * `_withdraw()`: Only emit `ReserveUsedAsCollateralDisabled` if `user.isUsingAsCollateral()` was `true` before. * `finalizeTransfer()`: * Only emit `ReserveUsedAsCollateralDisabled` if `user.isUsingAsCollateral()` was `true` before. * Only set `toConfig.setUsingAsCollateral(reserveId, true)` if the reserve represents a collateral asset. * `LendingPoolConfigurator`: * `updateVariableDebtToken()`: Validate that `input.asset` is not a collateral asset. * `configureReserveAsCollateral()`: * If `asset` is a collateral asset, then validate that `ltv`, `liquidationThreshold` and `liquidationBonus` are non-zero. * It `asset` is not a collateral asset, then validate that `ltv`, `liquidationThreshold` and `liquidationBonus` are zero. * `ValidationLogic` * `validateSetUseReserveAsCollateral()`: Validate that `reserve` represents a collateral asset if `useAsCollateral` is `true`.

Several critical functions on the `LendingPool` contract are only callable by vaults and can be used to remove an arbitrary amount of any asset. These functions include `LendingPool.withdrawFrom()` and `LendingPool.getYield()`. However, both of these functions allow any vault to remove any asset from the `LendingPool` as seen on `LendingPool.sol#L196:require(_availableVaults[msg.sender] == true, Errors.VT_PROCESS_YIELD_INVALID);`. This violates the principle of least privilege as vaults should only be able to remove assets they deal with. For example, the `LidoVault` should only be able to withdraw stETH from the `LendingPool`. Currently, if even one vault was compromised, it could be used to drain the entire `LendingPool` of all assets. We also note that any vault can deposit any asset through the `LendingPool.deposit()` and `LendingPool.depositYield()` functions. While the impact of this is less obvious, vaults should only be able to deposit assets that they deal with.

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.