Description

Vault is vulnerable to an ERC-4626--style inflation attack.

In accordance with ERC-4626, Vault is a vault that holds assets on behalf of its users, and whenever a user deposits assets, it issues to the user a number of shares such that the proportion of the user's shares over the total issued shares is equal to the user's assets over the total withdrawable assets. This allows assets gained by Vault to increase the value of every user's shares in a proportional way.

ERC-4626 vaults are susceptible to inflation attacks; however, an attacker can "donate" funds to the vault without depositing them, increasing the value of a share unexpectedly. In some circumstances, including when an unsuspecting user is the first depositor, an attacker can make back more than they donated, stealing value from the first depositor.

Impact

We created a proof of concept (POC) for this bug (section ). In this POC, a vault is empty (has no coin balance and zero issued shares), and then a benign user submits a transaction depositing 1,000 coins to the mempool.

Before the deposit transaction is mined, an attacker front-runs it with an earlier transaction, which deposits 0.000001 coins and then donates 1,000 coins to the vault. After this, the attacker has one share and the vault has 1,000.000001 coins.

Then, the user's deposit transaction is mined. After the user's deposit, the vault has 2,000.000001 coins, of which 1,000 was just deposited by the user. Since shares are now worth 1,000.0000005 coins after the attacker's front-run transactions, the user is given less than one share, which the vault rounds to zero.

Finally, the attacker, with their one share that represents all the issued shares, withdraws all of the assets, stealing the user's coins.

An excerpt of the POC output is shown below:

start

--- state ---
user shares: 0
user balance: 100000.0
attacker shares: 0
attacker balance: 100000.0
-------------

user signs tx depositing 1000, tx in mempool seen by attacker
attacker frontruns with a deposit of 0.000001 and a donation of 1000

--- state ---
user shares: 0
user balance: 100000.0
attacker shares: 1
attacker balance: 98999.999999
-------------

user deposit of 1000 occurs

--- state ---
user shares: 0
user balance: 99000.0
attacker shares: 1
attacker balance: 98999.999999
-------------

attacker withdraws all coins

--- state ---
user shares: 0
user balance: 99000.0
attacker shares: 0
attacker balance: 101000.08684
-------------

Recommendations

Please see Github issue #3706↗ in OpenZeppelin for discussion about how to mitigate this vulnerability.

In short, the first deposit to a new Vault could be made by a trusted admin during Vault construction to ensure that totalSupply remains greater than zero. However, this remediation has the drawback that this deposit is essentially locked, and it needs to be high enough relative to the first few legitimate deposits such that front-running them is unprofitable. Even if this prevents the attack from being profitable, an attacker can still grief legitimate deposits with donations, making the user gain less shares than they should have gained.

Another solution is to track totalAssets internally, by recording the assets gained through its Market positions and not increasing it when donations occur. This makes the attack significantly harder, since the attacker would have to donate funds by affecting price feeds for the underlying assets rather than just sending tokens to the Vault.

Remediation

This finding was acknowledged and a fix was implemented in commit .