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

The `SmartVaultV3.swap` function accepts any slippage, leading to the potential theft of all collateral tokens from the contract.

PTolev

Summary

The SmartVaultV3.swap function lacks slippage protection, making it vulnerable to sandwich attacks, potentially leading to the loss of swapped locked collateral tokens.

Vulnerability Details

In some cases the SmartVaultV3.swap() function uses amountOutMinimum and sqrtPriceLimitX96 set to 0, enabling MEV or sandwich attacks.

From Uniswap's documentation:

  • amountOutMinimum is advised to be non-zero in production to safeguard against price manipulation.

  • sqrtPriceLimitX96 being zero deactivates this parameter. In production, it should be set to manage price impact and facilitate other price-related mechanisms.

The SmartVaultV3.swap() function, callable by the vault owner, internally calls calculateMinimumAmountOut to calculate amountOutMinimum based on collateral value and swap token value. If the locked collateral tokens' value - the swapped token value exceeds requiredCollateralValue, the function returns 0. For example, this can happen when the ETH price bumps. This lack of slippage protection is critical, particularly when the value of tokens like ETH rises sharply.

A potential attack involves using a flash loan to manipulate the WETH/USDC pool on Uniswap, allowing the attacker to drain all USDC from the SmartVaultV3 contract. This method can also target other ERC-20 tokens in the contract.

For instance, to drain all USDC, consider the following proof-of-concept:

  • The attacker borrows a flash loan in USDC and buys WETH from Uniswap's WETH/USDC pool.

  • The attacker executes the SmartVaultV3.swap function with USDC.

  • The SmartVaultV3.swap will use all the locked USDC to buy WETH at a very high price.

  • The attacker then sells the previously obtained WETH for USDC in the same pool and repays the flash loan.

  • The attacker takes all the locked USDC as profit.

Coded Proof of Concept

Place this test in the smartVault.js file and execute it by POC: slippage, amountOutMinimum = 0

it('POC: slippage, amountOutMinimum = 0', async () => {
// user vault has 1 ETH collateral
await user.sendTransaction({to: Vault.address, value: ethers.utils.parseEther('1')});
// user borrows 1200 EUROs
const borrowValue = ethers.utils.parseEther('1200');
await Vault.connect(user).mint(user.address, borrowValue);
​
// price of eth increased
await ClEthUsd.setPrice(BigNumber.from(400000000000));
​
const inToken = ethers.utils.formatBytes32String('ETH');
const outToken = ethers.utils.formatBytes32String('sUSD');
​
// user is swapping .5 ETH
const swapValue = ethers.utils.parseEther('0.5');
await Vault.connect(user).swap(inToken, outToken, swapValue);
​
const { amountOutMinimum } = await SwapRouterMock.receivedSwap();
​
expect(amountOutMinimum).to.equal(0);
});

Furthermore, the deadline parameter is set to block.timestamp, allowing token swaps at any block number without an expiration deadline.
Another concern is the constant pool fee of 3000 (0.3%). This inflexibility could lead to liquidity shifts and higher MEV attack risks due to lower liquidity tiers.

Impact

This vulnerability, particularly during a significant price increase of locked collateral tokens, allows an attacker to deplete the SmartVaultV3 of various cryptocurrencies. It's considered a high-risk issue due to specific required conditions but can lead to substantial financial losses.

Additionally, without an expiration deadline, miners or validators could manipulate transactions for their benefit, increasing the risk of fund loss due to slippage.

Tools Used

Manual Review

Recommendations

To mitigate this vulnerability, it's advised not to use 0 as amountOutMinimum during swaps.
Given that the swap function is restricted to the vault owner, they should be able to set parameters like amountOutMinimum, sqrtPriceLimitX96, deadline, and fee to enhance security and control slippage.

- function swap(bytes32 _inToken, bytes32 _outToken, uint256 _amount) external onlyOwner {
+ function swap(bytes32 _inToken, bytes32 _outToken, uint256 _amount, uint256 _amountOutMinimum, uint256 _sqrtPriceLimitX96, uint256 _deadline, uint256 _fee) 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,
+ fee: _fee,
recipient: address(this),
- deadline: block.timestamp,
+ deadline: _deadline,
amountIn: _amount - swapFee,
- amountOutMinimum: minimumAmountOut,
+ amountOutMinimum: _minimumAmountOut,
- sqrtPriceLimitX96: 0
+ sqrtPriceLimitX96: _sqrtPriceLimitX96
});
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
Validated
Assigned finding tags:

deadline-check

hardcoded-fee

Support

FAQs

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