Medium Risk
https://github.com/Cyfrin/2023-12-the-standard/blob/91132936cb09ef9bf82f38ab1106346e2ad60f91/contracts/SmartVaultV3.sol#L206-L212
The code employs an on-chain slippage calculation mechanism through the calculateMinimumAmountOut
function, utilized within the swap
function. Consequently, this function may return 0
when the collateralization is deemed adequate.
return collateralValueMinusSwapValue >= requiredCollateralValue ?
0 : calculator.eurToToken(getToken(_outTokenSymbol), requiredCollateralValue - collateralValueMinusSwapValue);
The absence of slippage checks and minimum return amount validations in the code could result in trades occurring at suboptimal prices, potentially leading to the reception of fewer tokens than would be expected at prevailing fair market rates. This vulnerability might expose the vault owner to risks of incurring losses due to unfavorable prices at the time of trade execution. Because the swap
function will call ExactInputSingleParams
function with amountOutMinimum
set to 0
:
function swap(bytes32 _inToken, bytes32 _outToken, uint256 _amount) external onlyOwner {
uint256 swapFee = _amount * ISmartVaultManagerV3(manager).swapFeeRate() / ISmartVaultManagerV3(manager).HUNDRED_PC();
address inToken = getSwapAddressFor(_inToken);
uint256 minimumAmountOut = calculateMinimumAmountOut(_inToken, _outToken, _amount);
ISwapRouter.ExactInputSingleParams memory params = ISwapRouter.ExactInputSingleParams({
tokenIn: inToken,
tokenOut: getSwapAddressFor(_outToken),
fee: 3000,
recipient: address(this),
deadline: block.timestamp,
amountIn: _amount - swapFee,
amountOutMinimum: minimumAmountOut,
sqrtPriceLimitX96: 0
});
inToken == ISmartVaultManagerV3(manager).weth() ?
executeNativeSwapAndFee(params, swapFee) :
executeERC20SwapAndFee(params, swapFee);
}
The risk associated with the absence of slippage and minimum return amount checks lies in potential price volatility during the swap. Trades can happen at a bad price and lead to receiving fewer tokens than at a fair market price.
Manual review.
Ensure that users are allowed to specify their own slippage parameters which were calculated on their own e.g off-chain.
Should be invalid because this is a known issue in the README file. minimumAmountOut can be set to 0 if there is no value required to keep collateral above required level. The user is likely to lose some value in the swap, especially when Uniswap fees are factored in, but this is at the user's discretion. Shows no way of attacking that isn't at users discretion.
While this is true that the swap implementation allows for slippage as mentioned above, the contest page mentions loss of the user loss is to be expected and the swap function is only designed to check for value sufficient to keep it collateralized. The fair interpretation of this is that the limitations described above are known.
While this is true that the swap implementation allows for slippage as mentioned above, the contest page mentions loss of the user loss is to be expected and the swap function is only designed to check for value sufficient to keep it collateralized. The same was confirmed by the sponsor. The fair interpretation of this is that the limitations described above are known.