search

sherlock-audit / 2024-08-sentiment-v2-judging

5 stars 5 forks source link

ajayss - initial depositor will use inflation attack of ERC4626 to steal subsequent deposit assets #603

Closed sherlock-admin3 closed 2 months ago

sherlock-admin3 commented 3 months ago

ajayss

Medium

initial depositor will use inflation attack of ERC4626 to steal subsequent deposit assets

Summary

The initial deposit problem of ERC4626 allows the first depositor to deposit 1 asset to get 1 share. Then a victim's transaction is seen in the mempool and front-run with a transfer transaction to increase the totalAssets amount such that the victim gets one share

        shares = _convertToShares(assets, pool.totalDepositAssets, pool.totalDepositShares, Math.Rounding.Up);

        //~ in _convertToShares
        shares = assets.mulDiv(totalShares, totalAssets, rounding);

After the victim deposits his assets, the attacker withdraws his share netting his asset and the victim's assets

Root Cause

In Pool.sol:310 The case of inflation attacks hasn't been dealt with. For example uniswap sends the first 1000 shares to the zero address. https://github.com/sherlock-audit/2024-08-sentiment-v2/blob/main/protocol-v2/src/Pool.sol#L315

Internal pre-conditions

  1. The attacker needs to call deposit to get 1 share
  2. The victim transaction to deposit is in the mempool
  3. attacker sees this and sends the right amount of assets to make sure victim gets just one share (by inflating totalAssets)
  4. The victim gets 1 share
  5. The attacker withdraws his share and nets a decent chunk of the victim's assets

External pre-conditions

None

Attack Path

  1. The hacker back-runs a transaction of an ERC4626 pool creation.
  2. The hacker mints for themself one share: deposit(1). Thus, totalAsset()==1, totalSupply()==1.
  3. The hacker front-runs the deposit of the victim who wants to deposit 20,000 USDT (20,000.000000).
  4. The hacker inflates the denominator right in front of the victim: asset.transfer(10_000e6). Now totalAsset()==10_000e6 + 1, totalSupply()==1.
  5. Next, the victim's tx takes place. The victim gets 1 * 20_000e6 / (10_000e6 + 1) == 1 shares. The victim gets only one share, which is the same amount as the hacker has.
  6. The hacker burns their share and gets half of the pool, which is approximately 30_000e6 / 2 == 15_000e6, so their profit is +5,000 (25% of the victim's deposit).

Impact

No response

PoC

No response

Mitigation

Any of the popular defenses https://blog.openzeppelin.com/a-novel-defense-against-erc4626-inflation-attacks

Duplicate of #26

neko-nyaa commented 2 months ago

Escalate.

Not a dupe of #26, but rather dupe of the 4626 inflation attack, which has been mentioned in a few other issues

sherlock-admin3 commented 2 months ago

Escalate.

Not a dupe of #26, but rather dupe of the 4626 inflation attack, which has been mentioned in a few other issues

You've created a valid escalation!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

cvetanovv commented 2 months ago

This is a known issue. Check this from the previous report: https://github.com/sentimentxyz/protocol-v2/blob/master/audits/sentiment_v2_zobront.md#m-02-erc4626-is-vulnerable-to-donation-attacks

According to the readme:

Previously acknowledged issues from past audits must be considered acceptable risks.

Planning to reject the escalation and leave the issue as is.

Evert0x commented 2 months ago

Result: Invalid Duplicate of #26

sherlock-admin2 commented 2 months ago

Escalations have been resolved successfully!

Escalation status: