The Standard

Summary

Protocol allows user's swap to lose almost all of the tokens during swap as long as user is collateralized. The reasoning behind that is that, even though user loses some tokens after swap, he will still be collateralized enough not to produce bad debt for the protocol. However user can be harmed in such scenario and current design should not be used in production.

Vulnerability details

The fact that user will be collateralized after swap is correct however there is no point in setting slippage to 0. Zero slippage can harm user's trade to a point he could lose most of the tokens. In some scenarios like low liquidity in a pool (LINK/WBTC 0xa79fD76cA2b24631Ec3151f10c0660a30Bc946E7 0,3% fee) and zero slippage, user can receive very bad price for his tokens.

With zero slippage in the current desing there is no incentive for a user to use swap function provided in SmartVaultV3. He can possibly lose money on trade and also has to pay swapFee to a protocol for using swap function. Knowing that there is no MEV on arbitrum, there is still a possibilty that user's transaction could get placed after huge swap in unfavorable derection thus bad price. Better option for a user could be withdrawing tokens from vault, trading tokens himself with his custom parameters that he tolerates and deposit the tokens afterwards to the vault.

As shown in code snippet above the slippage calcuation are based on the required collateral and the value of the collateral minus swap value.

When user wishes to trade less than required collateral, the slippage will be set to 0. In other scenario, when user would like to trade more than required collateral the slippage will be calculated as requiredCollateralValue - collateralValueMinusSwapValue.

1. First scenario, slippage set to 0.

• minted = 100 euro

• collateralRate = 120 %

• euroCollateral() = 150 euro (overcollateralized by 30 euro)

User wished to trade 20 euro worth of collateral.

After calculations:

• requiredCollateralValue = 100 euro * 120% = 120 euro

• collateralValueMinusSwapValue = 150 euro - 20 euro = 130 euro

collateralValueMinusSwapValue (130 euro) > requiredCollateralValue (120 euro) it means that slippage will be set to zero and user could receive very little amount of tokens from this swap and transaction will still go through.

2. Second scenario, slippage set to equivalent value of euro in tokens from equation requiredCollateralValue - collateralValueMinusSwapValue.

• minted = 100 euro

• collateralRate = 120 %

• euroCollateral() = 150 euro (overcollateralized by 30 euro)

User wished to trade 60 euro worth of collateral.

After calculations:

• requiredCollateralValue = 100 euro * 120% = 120 euro

• collateralValueMinusSwapValue = 150 euro - 60 euro = 90 euro

requiredCollateralValue (120 euro) > collateralValueMinusSwapValue (90 euro) it means that the slippage will be set to 30 euro worth of tokens which means that user could lose up to 50% of his trade.

Current implementation protects protocol from users that could try to undercollateralize their postions through swap however current design could also harm users. As mentioned before, there is no front-running on Arbitrum One but since there is a compatibilities section (Any EVM chains with live Chainlink data feeds and live Uniswap pools) i would like to mention that this design is even more vulnerable to losing funds on chains like Ethereum due to sandwitch attacks which are common on Mainnet.

Impact

User can lose money on trade due to zero slippage. Disincentivizing user's by implementing trades with bad slippage could result in loses for protocol as user will not user their swap and use other protocols providing better prices and more customizable swaps.

Tools used

VScode, Manual Review

Recommendations

To fix this issue the protocol could change calculateMinimumAmountOut logic in a way that, when collateralValueMinusSwapValue > requiredCollateralValue allow user to set his own slippage. When requiredCollateralValue > collateralValueMinusSwapValue require that the slippage provided by user is greater than requiredCollateralValue. This will protect protocol from users trying to undercollateralize thier vault through swaps and protect users from bad prices on uniswap.