-
Notifications
You must be signed in to change notification settings - Fork 11.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Preventing vault price reset in ERC4626 #3800
Comments
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? |
I think that could be also an option if the additional storage cost is acceptable. |
This comment was marked as spam.
This comment was marked as spam.
Fixed by #3979. |
🧐 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:
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).
The text was updated successfully, but these errors were encountered: