The Standard

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

Vault collateral can be lost due to dangerous configuration of swap parameters

Description

SmartVaultV3::swap provides functionality to allow vault owners to swap one collateral token for another. The use of block.timestamp as the swap deadline is not too severe an issue in isolation but does mean that the transaction can be withheld by validators and included at any arbitrary point in the future, at which point market conditions could have changed, and so does become problematic when used in conjunction with misconfigured slippage parameters such as sqrtPriceLimitX96. As shown below, this slippage parameter is hardcoded to zero, which is especially dangerous if amountOutMinimum is also zero.

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);
}

An important check is performed by SmartVaultV3::calculateMinimumAmountOut, which validates the vault will remain fully collateralized as a result of the swap and returns the minimum amount necessary to ensure this is the case. This value is the minimum output token amount as required by the protocol; however, it is used in the context of a third-party contract where it has other implications as a slippage parameter. Also note that contrary to the statement in the known issues, that minimumAmountOut can be zero at the user's discretion, vault owners have no direct control over the value of minimumAmountOut.

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

This behavior is such that if the swap will not result in the vault becoming undercollateralized, the protocol deems it permissible to lose the entirety of the swap value. Considering all the points above, the vault owner will lose the entire delta between their true swap value and the minimum output amount due to sandwich attacks.

If the owner is attempting to swap collateral that is backing outstanding debt, SmartVaultV3::calculateMinimumAmountOut intends to configure the slippage parameter via the else branch of the ternary operator such that the output of the swap is guaranteed to cover only this obligation and no more; however, this calculation is not performed correctly and so will result in the vault becoming immediately liquidatable, as demonstrated below. This is clearly undesirable and not the intended behavior.

Impact

The output value of vault collateral swaps is highly likely to be lost to sandwich attacks, potentially also resulting in erroneous liquidations, so this issue is of high severity.

Proof of Concept

Considering a collateral rate of :

  1. Alice deposits 1000 EUR in LINK into her vault.

  2. maxMintable is 833 EURO.

  3. Alice mints 400 EURO against her collateral.

  4. Swaps:
    i. Alice swaps 200 EUR in LINK for ARB.
    ii. Alice swaps 800 EUR in LINK for ARB.

  5. Outputs:
    i. Alice receives 0 ARB.
    ii. Alice receives 200 EUR in ARB.

  6. Results:
    i. Alice loses 200 EUR in collateral, but her vault remains fully collateralized: ; ; .
    ii. Alice loses 600 EUR in collateral, and her vault is now undercollateralized: ; ; .

As shown above, when the swap value exceeds the collateral requirement, the minimum output amount from the swap is not correctly calculated and will result in the vault becoming undercollateralized.

Recommended Mitigation

  • Separate collateralization validation from the calculation of the swap slippage parameters and consider allowing the vault owner to specify a value for minimumAmountOut.

  • Fix the calculation in SmartVaultV3::calculateMinimumAmountOut to correctly ensure that it is not possible for a vault to become immediately liquidatable as a result of a swap between two collateral tokens.

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.