openzeppelin-contracts
openzeppelin-contracts copied to clipboard
OpenZeppelin
Preventing vault price reset in ERC4626
🧐 Motivation
New security feature recommendation preventing the ERC4626 vault price reset.
📝 Details
I'm concerned with the possibility of ERC4626 vaults being emptied during their lifecycle. This is because, once a vault is emptied, the conversion rate of shares (or the share price) is reset to the initial value (which is currently 1:1 by default). The change in the share price could be significant especially when the vault has generated a high yield until the reset (in which case the share price will be drastically decreased).
The sudden change in share price (especially the big price drop) could be a problem and may be exploited in some ecosystem where the vault is used as price oracle for other protocols.
A potential exploit scenario is as follows:
- Adversaries create a vault, deposit a good amount of initial assets, and inflate the share price by "donating" more assets or generating a (fake) yield. (Say, the initial deposit is 10 ETH and the fake yield is 40 ETH, thus the inflated price is 5 = 50/10.)
- They register their vault for some lending protocols that admit arbitrary ERC4626 vault tokens as collateral and use the vault share's conversion rate as price oracle for their TWAP.
- Then, they wait until the lending protocols' TWAP fully reflects the inflated price.
- Then, they atomically withdraw all the assets from the vault (which immediately resets the conversion rate to the initial value 1:1), mint a lot of shares with the initial rate 1:1, and use them as collateral to borrow much more from the lending protocols while their TWAP is still high reflecting the inflated price. (Say, they mint 100 shares by depositing only 100 ETH after the vault is emptied, and use the 100 shares as collateral to borrow 250 ETH (assuming 200% collateralization ratio). This is possible because the 100 shares is valued at ~500 ETH at the moment since the TWAP is still (close to) 5 because it doesn't quickly reflect the sudden price drop. Then, the adversaries walk away with making a profit of 150 ETH.)
Recommendation
You may add a requirement that, once a vault is initialized (and funded), the totalSupply must remain to be greater than a certain threshold (say 1 gwei = 10^9). This way, the conversion rate will be kept being track of, even in the case that (almost) all the shares have been redeemed.
This requirement could be made optional if it is too restrictive for some use cases.
Remark
Note that the above requirement implies the minimum initial deposit being greater than the threshold, which can also mitigate the price inflation front-running attack issue #3706.
Also note that step (1) in the above scenario is feasible even if the vault explicitly tracks of totalAssets, since the adversaries can still utilize the yield generating mechanism to increase totalAssets legitimately (in a similar way as described in https://www.rileyholterhus.com/writing/bunni by @rholterhus).
Thanks, this is interesting and we will consider it along with #3706. (We're actively discussing mitigations for #3706 and will be acting on that in the next 2 weeks.)
In the exploit described the adversary is the vault deployer so even if we implement a mitigation for this issue it's still something that ERC4626 integrators should consider in their threat model, as the adversary can simply use a non-OpenZeppelin implementation. An important exception for this is if the vault deployer is a factory (the vulnerable contract might even automatically whitelist all vaults created by the factory), in which case the adversary could be a user of the factory and that is something that we would want to protect our users against.
Would it be an option to preserve the prior share price after the vault is emptied, instead of resetting to 1:1?
Would it be an option to preserve the prior share price after the vault is emptied, instead of resetting to 1:1?
I think that could be also an option if the additional storage cost is acceptable.
