First depositor can abuse exchange rate to steal funds from later depositors

[hats-bug-reporter[bot]]

Github username: -- Twitter username: -- Submission hash (on-chain): 0x46368fbc842f2b640f1c43dbb71dba60b82a61d3bc832accebac850fb667eebe Severity: high

Description: First depositor can abuse exchange rate to steal funds from later depositors Classic issue with vaults. First depositor can deposit a single wei then donate to the vault to greatly inflate share ratio. Due to truncation when converting to shares this can be used to steal funds from later depositors.

Attack Scenario\ Attacker can first deposit small amount of loan token to get pool tokens, and front-run other depositors' transactions and inflate pool token price by large "donation", thus attacker can withdraw more loan tokens than he initially owned.

1. Alice deploys the pool contract and submits a transaction to deposit 2 ether.

2. Bob monitors the mempool and sees Alice’s pending deposit transaction.

3. Bob front-runs Alice's transaction by first depositing 1 wei to the pool, which allows him to receive 1 pool token.

4. Bob artificially inflates the pool token price by transferring 1 ether loan token directly to the pool, increasing the token value to 1 ether + 1 wei.

5. Alice's deposit transaction is confirmed, and she receives only 1 pool token (due to the inflated price).

6. Bob withdraws from the pool and receives 1.5 ether loan tokens back, resulting in a profit of 0.5 ether. Attachments

Impact

User's deposited loan tokens may be stolen by attacker.

1. Proof of Concept (PoC) File

   
   /**
    * @notice Deposits the specified amount of assets (USDE) to receive the corresponding number of shares (EUI).
    * @param assets The amount of assets (USDE) to be deposited.
    * @param receiver The address that will receive the shares (EUI).
    * @return shares The number of shares (EUI) received.
    */
   function deposit(uint256 assets, address receiver) public returns (uint256 shares) {
       shares = convertToShares(assets);
       usde.burn(msg.sender, assets);
       _mint(receiver, shares);
   
       emit Deposit(msg.sender, receiver, assets, shares);
   }

## How to resolve this classic issue
Consider minting a minimal amount of pool tokens during the first deposit and sending them to zero address, this increases the cost of the attack. [Uniswap V2](https://app.uniswap.org/whitepaper.pdf)  uses the value 1000 as it is small enough to don't hurt the first minter, while still increasing the cost of this attack by 1000x.

[AndreiMVP]

I don't think this is relevant, price is determined by YieldOracle

[catellaTech]

Hey, @AndreiMVP.

Thank you for your response. I understand that the price is determined by the YieldOracle, but I’d like to highlight that this vulnerability is a common issue in ERC-4626 vaults, even with a yield oracle, and it particularly affects the initial deposit flow.

The problem lies in the fact that the first depositor can exploit the pool’s price calculation. The attack involves the first user depositing a small amount to obtain pool tokens and then artificially inflating the value of those tokens through a substantial “donation.” This increases the token price ratio, which negatively impacts later depositors, as they receive fewer tokens for their funds, allowing the attacker to withdraw more than they initially deposited.

Even with a YieldOracle, the initial price calculation remains susceptible because the oracle doesn’t adjust the price in real-time to correct for the large variations caused by an artificial donation. This type of attack is a known exploit, and platforms like Uniswap V2 implement specific defenses to mitigate such attacks, such as minting a small number of tokens at the outset and sending them to the zero address. This significantly raises the cost of manipulating the initial token value in the pool.

Implementing a similar solution (e.g., minting 1,000 pool tokens on the first deposit and burning them) would deter manipulation of the initial value and reduce the feasibility of the attack without affecting legitimate users.

Thank you for considering this matter. I’m available to provide further technical details if needed.

Have a good day!

[catellaTech]

check this blog to learn more about the issue https://blog.openzeppelin.com/a-novel-defense-against-erc4626-inflation-attacks

[AndreiMVP]

I am aware of the ERC4626 exploits that you mention but this protocol is not working exactly according to the common standard, just implementing the interface. In the case of the YieldOracle price, it is calculated offchain and it shouldn't run into issues with initial deposit flow.

[catellaTech]

I understand that the protocol follows the ERC-4626 interface, but the core issue here arises from the implementation of the vault logic itself, which doesn’t fully account for the risks associated with the share and asset handling in the context of an ERC-4626 vault.

While your protocol adheres to the interface, the inflation attack risk persists because the logic behind how shares are issued and the assets are calculated during the deposit flow does not adequately address the potential manipulation of the share-to-asset ratio. The vulnerability is particularly evident during the initial deposit flow, where an attacker can exploit the system by making a small deposit, then donating a larger amount to inflate the vault's balance. This causes the first depositor's share to be diluted, which could lead to significant losses for legitimate users.

Even though the YieldOracle is used for price calculation offchain, the key issue is still related to how shares and assets are handled within the vault itself. The standard ERC-4626 framework does not inherently address this specific risk, and additional precautions are needed in the vault logic to prevent inflation attacks that manipulate the distribution of shares.

This is a well-known issue with ERC-4626 vaults, and protocols like Uniswap V2 have addressed similar concerns by implementing safeguards such as minting an initial set of "dead shares" or similar strategies to mitigate the risk of inflation attacks during the first deposit.

To summarize, while following the ERC-4626 interface is a solid foundation, the vault’s internal logic should be adjusted to account for these vulnerabilities, ensuring fair share distribution and protecting users from the potential exploits that arise from manipulating the deposit flow.

[catellaTech]

check this blog to learn more about the issue https://blog.openzeppelin.com/a-novel-defense-against-erc4626-inflation-attacks

Here is a lot of examples how big protocols addresses this issue. You did not take it into account in your implementation.

[catellaTech]

I am aware of the ERC4626 exploits that you mention but this protocol is not working exactly according to the common standard, just implementing the interface. In the case of the YieldOracle price, it is calculated offchain and it shouldn't run into issues with initial deposit flow.

No make sense that you said this but in the contract you guys make clear this:

/// ---------- ERC4626 FUNCTIONS ---------- ///

[catellaTech]

How this issue impact other user:

https://gist.github.com/catellaTech/2e4be5072a497697c44f9c23e45b5b6a