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

The calculations in ``calculateMinimumAmountOut()`` will put the user utilizing a swap function in danger of a sandwich attack

dimulski

Summary

The calculations in calculateMinimumAmountOut() will put the user utilizing a swap function in danger of a sandwich attack. If he gets sandwiched for a big amount and thus lose a big part of his liquidity his SmartVault can even get liquidated. As there is no manual way to set the minimumAmountOut a user doesn't have an option to protect himself from being sandwiched if he want to utilize the swap function. If a user tries to make a swap where the collateralValueMinusSwapValue >= requiredCollateralValue in the calculateMinimumAmountOut() function is true, this makes things even worse as this will set the minAmountOut to 0. The parameters supplied in the below POC are not some magic values which showcase an edge case, whatever the amountIn put in the swap() function the minAmountOut will always be too low, and thus unsafe for the user to execute a swap. The bigger the amount, the bigger the danger of a user getting liquidated after he gets sandwiched.

Vulnerability Details

Gist

After executing the steps provided in the above gist in order to set up the tests, add the following functions to the AuditorTests.t.sol

function test_SwapMinAmountOutWorksIncorectly() public {
skip(2 days);
vm.startPrank(alice);
(, int256 EurUsdRate,,,) = ClEurUsd.latestRoundData();
(, int256 EthUsdRate,,,) = ClEthUsd.latestRoundData();
uint256 amountInETH = (150_000e18 * uint256(EurUsdRate)) / uint256(EthUsdRate);
vm.deal(alice, amountInETH);
(address vault, ) = vaultManagerV5Instance.mint();
address(vault).call{value: amountInETH}("");
SmartVaultV3 vaultInstance = SmartVaultV3(payable(vault));
vaultInstance.mint(alice, 100_000e18);
uint256 amountInETHForSwap = (50_000e18 * uint256(EurUsdRate)) / uint256(EthUsdRate);
console2.log("Here is the eth amount for the swap: ", amountInETHForSwap/1e18);
console2.log("Alice EURO balance: ", EURO.balanceOf(alice));
uint256 minAmountOut = vaultInstance.calculateMinimumAmountOut(ethBytes32, ethBytes32, amountInETHForSwap);
console2.log("The min amount output: ", minAmountOut/1e18);
vm.stopPrank();
}

For this test I have added the following console.logs to the SmartVaultV3.sol contract, I have modified the calculateMinimumAmountOut() function to this

function calculateMinimumAmountOut(bytes32 _inTokenSymbol, bytes32 _outTokenSymbol, uint256 _amount) public view returns (uint256) {
ISmartVaultManagerV3 _manager = ISmartVaultManagerV3(manager);
uint256 requiredCollateralValue = minted * _manager.collateralRate() / _manager.HUNDRED_PC();
console2.log("From contract requiredCollateralValue: ", requiredCollateralValue);
console2.log("From contract tokenToEur: ", calculator.tokenToEur(getToken(_inTokenSymbol), _amount));
uint256 collateralValueMinusSwapValue = euroCollateral() - calculator.tokenToEur(getToken(_inTokenSymbol), _amount);
console2.log("From contract collateralValueMinusSwapValue: ", collateralValueMinusSwapValue);
return collateralValueMinusSwapValue >= requiredCollateralValue ?
0 : calculator.eurToToken(getToken(_outTokenSymbol), requiredCollateralValue - collateralValueMinusSwapValue);
}

I have also imported this in the SmartVaultV3.sol contract
import {Test, console2} from "forge-std/Test.sol";

Logs:
Here is the eth amount for the swap: 23
Alice EURO balance: 100000000000000000000000
From contract requiredCollateralValue: 120600000000000000000000
From contract tokenToEur: 49999999999999999998269
From contract collateralValueMinusSwapValue: 100000000000000000000875
The min amount output: 9

I have used the same output and input tokens to better illustrate the difference between the minAmoutOut and the amount of input token. As we can se from the above logs if a user want to swap 23e18ETH the minAmountOut outputted by calculateMinimumAmountOut() will be 9e18ETH which is a difference of 23-9 = 14e18ETH which the user can lose due to sandwich attack.

To run the test use: forge test -vvv --mt test_SwapMinAmountOutWorksIncorectly

Impact

If a user decides to swap some of the assets he has as collateral, the calculations in calculateMinimumAmountOut() will set too low minAmountOut which will make him a victim of a sandwich attack, which can even lead to the liquidation of his SmartVault

Tools Used

Manual review & Foundry

Recommendations

Remove the calculateMinimumAmountOut() function, and let the user set the desired minimumAmountOut manually

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.