The Standard
The Standard
DeFiHardhat
20,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

Missing slippage protection in the swap function in Smart Vault for the case where min amount is 0 can lead to loss of funds during swap

SolSaver

Summary

In SmartVaultV3::swap(), the user has no control over amountOutMinimum that is used for the swap. The value of amountOutMinimum can be 0, which can lead to loss of funds because of swap.

In the known issues it is mentioned that:

- `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

However, the user has no real discretion for the cases where the amountOutMinimum is 0, and the user should be given control of amountOutMinimum in such case.

Vulnerability Details

In SmartVaultV3::swap(), the amountOutMinimum is determined to ensure there is enough collaterization after swap. The value of amountOutMinimum can either be 0 if there is enough collaterization already, or some other amount that is determined to ensure there is enough collaterization.

For the case where there is enough collaterization, and amountOutMinimum is set to 0, the user's only option is to perform a swap with amountOutMinimum as 0.

In the known issues it is mentioned:

- `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

But "is at the user's discretion" is not true anymore if the user's only option is to swap at amountOutMinimum as 0, and that they have no control in such cases.

Tools Used

Manual Review

Recommendations

Take in a value for amountOutMinimum from the user, and use this value for the cases where amountOutMinimum is 0.

Also, in the other case where amountOutMinimum is not zero, if the user provides a larger value, then that larger value can be used instead.

- function swap(bytes32 _inToken, bytes32 _outToken, uint256 _amount) external onlyOwner {
+ function swap(bytes32 _inToken, bytes32 _outToken, uint256 _amount, uint256 desiredAmountOutMinimum) external onlyOwner {
uint256 swapFee = _amount * ISmartVaultManagerV3(manager).swapFeeRate() / ISmartVaultManagerV3(manager).HUNDRED_PC();
address inToken = getSwapAddressFor(_inToken);
uint256 minimumAmountOut = calculateMinimumAmountOut(_inToken, _outToken, _amount);
+ if (desiredAmountOutMinimum > minimumAmountOut) {
+ minimumAmountOut = desiredAmountOutMinimum;
+ }
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);
}
Updates

Lead Judging Commences

hrishibhat Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

Slippage-issue

hrishibhat Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Known issue
Assigned finding tags:

Slippage-issue

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.