Skip to content
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

The price of fractionalToken can be manipulated such that the future liquidity providers loose their baseTokens #242

Closed
code423n4 opened this issue Dec 18, 2022 · 2 comments
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working duplicate-442 satisfactory satisfies C4 submission criteria; eligible for awards

Comments

@code423n4
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2022-12-caviar/blob/main/src/Pair.sol#L417

Vulnerability details

On first creating a new Pair, the user can define the amount of baseTokens and nfts they want to transfer to the contract. This is also true with an already initialized Pair, any user can select the amount of baseTokens and the amount of nfts they want to transfer in the contract.

An attacker can use this to manipulate price and steal from future liquidityProviders.

Vulnerability details

https://github.com/code-423n4/2022-12-caviar/blob/main/src/Pair.sol#L417

Proof Of Concept

  1. Attacker adds 1wei and 100BAYC to create a new Pair of BAYC vs ETH.
  2. The price of pair is highly manipulated right now, and consists of 1wei : 100BAYC.
  3. Now a victim (alice) adds liquidity of 100 ETH and 100 bayc tokens.
  4. The pool now consists of ~100ETH : 200BAYC.
  5. The attacker would now remove their liquidity from the pool by selling their lpTokens and would get back ~50ETH and 100BAYC.
  6. The pool now has 50ETH and 100BAYC tokens, and the victim on removing their liquidity will get ~50ETH and 100BAYC having lost ~50ETH and attacker made a profit of ~50ETH.

Another attack vector would look like this:

  1. Attacker wraps 1M fractional tokens worth of nfts.
  2. Attacker initialises the pool with 1 basic unit of ETH and 1 basic unit of fractionalTokens.
  3. LP token supply right now would be 1 basic unit.
  4. Attacker then directly transfers 1M eth and 1M ( minus 1 basic unit) of fractionalTokens to the Pair contract.
  5. Now any future liquidity provider who try to add liquidity will get 0 LP tokens but their tokens would be transfered to the contract.
  6. Attacker can remove all the liquidity from the pool using 1 unit LP token that they have.

Altough there is a slippage check minLpTokenAmount which prevents it from happening, it would only be used when the user specifically puts the minimum LP amount they want to recieve. An unknowing user who is unaware of the price of LP token can be scammed by the attacker.

This is a issue similar to : https://code4rena.com/reports/2022-01-elasticswap/#m-01-the-value-of-lp-token-can-be-manipulated-by-the-first-minister-which-allows-the-attacker-to-dilute-future-liquidity-providers-shares and TOB-YEARN-003 ( https://docs.yearn.finance/security/ : trail of bits )

Recommendations:

I suggest using the same mechanism in uniswap v2 to prevent such attack by locking first few liquidity tokens.

@code423n4 code423n4 added 3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working labels Dec 18, 2022
code423n4 added a commit that referenced this issue Dec 18, 2022
@c4-judge
Copy link
Contributor

berndartmueller marked the issue as duplicate of #442

@c4-judge
Copy link
Contributor

berndartmueller marked the issue as satisfactory

@c4-judge c4-judge added the satisfactory satisfies C4 submission criteria; eligible for awards label Jan 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working duplicate-442 satisfactory satisfies C4 submission criteria; eligible for awards
Projects
None yet
Development

No branches or pull requests

2 participants