Users (who previously deposited assets) can withdraw and redeem their assets via functions withdraw() or redeem() at 1:1 ratio on a normal circumstance. Meaning, they are expecting an amount of at least equal to the amount that they have deposited.
However there will be occasions where their assets "withdrawals / redemptions" might be less than what they originally deposited. That scenario happens when the vault experiences losses.
function convertToAssets(uint256 shares) public view returns (uint256 assets) {
return Math.mulDiv(shares, totalAssets(), totalSupply);
}
Looking at the math expression above shares * totalAssets() / totalSupply(), when totalAssets() decreases, the return value (assets) also decreases.
Here's a scenario where the losses happen:
Suppose the vault is operating on a normal condition (without a loss).
Alice previously deposited an asset amount of 1000 and received 1000 amount of shares in return (1:1).
Now Alice is about to redeem the full amount and is expecting 1000 amount of assets in exchange of her 1000 shares.
While Alice's transaction is still pending in the mempool, the vault suddenly experience losses and the totalAssets() dropped by 10% below the original ratio of 1:1.
Now the transaction went through, she received 900 amount of assets (less than 10% of what she is expecting).
At this point, Alice realized her losses.
If only Alice knew that this will happen, she might have opted to wait until the assets goes back to at least 1:1 ratio to at least redeem back her original deposit amount. Without slippage protection, Alice has to suffer these losses.
This principle also applies to withdraw() but with a slight difference.
In redeem() the input (fixed) amount of shares is exchanged with assets. When losses occur, these fixed amount shares will produce lesser assets.
In withdraw() the input (fixed) amount of assets will cause to burn more shares to get the same asset amount which also in itself a loss.
function previewWithdraw(uint256 assets) public view returns (uint256 shares) {
uint256 supply = totalSupply; // Saves an extra SLOAD if totalSupply is non-zero.
return Math.mulDivRoundingUp(assets, supply, totalAssets());
}
Looking at the math expression above assets * supply / totalAssets(), when totalAssets() decreases, the return value (shares) increases pressuring to burn more shares.
_burn(owner, shares);
Impact
Without slippage protection on redeem() and withdraw() functions, users will lose funds in the event of vault loss that suddenly happens in the middle of the redeem / withdraw transactions.
Proof of Concept
Tools Used
Manual Review
Recommended Mitigation Steps
Implement a function with the same name redeem / withdraw (to still be compliant with ERC4626 but with different set of parameters) but add parameters minAssets and maxShares respectively.
Lines of code
https://github.com/code-423n4/2024-04-panoptic/blob/833312ebd600665b577fbd9c03ffa0daf250ed24/contracts/CollateralTracker.sol#L591-L626 https://github.com/code-423n4/2024-04-panoptic/blob/833312ebd600665b577fbd9c03ffa0daf250ed24/contracts/CollateralTracker.sol#L531-L566
Vulnerability details
Description
Users (who previously deposited assets) can withdraw and redeem their
assets
via functionswithdraw()
orredeem()
at 1:1 ratio on a normal circumstance. Meaning, they are expecting an amount of at least equal to the amount that they have deposited.However there will be occasions where their
assets
"withdrawals / redemptions" might be less than what they originally deposited. That scenario happens when the vault experiences losses.redeem()
previewRedeem()
convertToAssets()
Looking at the math expression above
shares * totalAssets() / totalSupply()
, whentotalAssets()
decreases, the return value (assets
) also decreases.Here's a scenario where the losses happen:
redeem
the full amount and is expecting 1000 amount of assets in exchange of her 1000 shares.totalAssets()
dropped by 10% below the original ratio of 1:1.If only Alice knew that this will happen, she might have opted to wait until the assets goes back to at least 1:1 ratio to at least redeem back her original deposit amount. Without slippage protection, Alice has to suffer these losses.
This principle also applies to
withdraw()
but with a slight difference.In
redeem()
the input (fixed) amount ofshares
is exchanged withassets
. When losses occur, these fixed amount shares will produce lesser assets.In
withdraw()
the input (fixed) amount ofassets
will cause to burn more shares to get the same asset amount which also in itself a loss.withdraw()
previewWithdraw()
Looking at the math expression above
assets * supply / totalAssets()
, whentotalAssets()
decreases, the return value (shares
) increases pressuring to burn more shares.Impact
Without slippage protection on
redeem()
andwithdraw()
functions, users will lose funds in the event of vault loss that suddenly happens in the middle of theredeem / withdraw
transactions.Proof of Concept
Tools Used
Manual Review
Recommended Mitigation Steps
Implement a function with the same name
redeem / withdraw
(to still be compliant with ERC4626 but with different set of parameters) but add parametersminAssets
andmaxShares
respectively.For
redeem()
For
withdraw()
Assessed type
Other