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

Frontrunning the first deposit may unbalance the pair #341

Closed
code423n4 opened this issue Dec 19, 2022 · 3 comments
Closed

Frontrunning the first deposit may unbalance the pair #341

code423n4 opened this issue Dec 19, 2022 · 3 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 upgraded by judge Original issue severity upgraded from QA/Gas by judge

Comments

@code423n4
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2022-12-caviar/blob/0212f9dc3b6a418803dbfacda0e340e059b8aae2/src/Pair.sol#L63

Vulnerability details

Impact

Creating the pool and making the first deposit are two separate actions. When the pool is first created, before the owner provides the first assets in a reasonable ratio, an attacker/griefer can attack the pool with an absurd ratio which will cause an imbalance between the assets in the pool.

Such an attack may cause two issues:

  • if the depositor that comes after the attacker tries to deposit with a reasonable ratio and chooses a small slippage, it can not deposit because of the slippage protection.
  • if the depositor that comes after the attacker tries to deposit with a reasonable ratio but does not choose a small slippage, the attacker can sandwich the depositor

Proof of Concept

Consider this scenario:

  1. A new pool is created for a pair of an NFT collection named X and USDT.
  2. Let's assume floor of NFT is around 1000 USDT, in that case it makes sense to start the pool with a nftAdd() with a single NFT and 1000e18 USDT. This will make the pool consist of 1000e18 USDT + 1e18 FracX(fractionised X NFT)
  3. If an attacker wraps their own NFT to 1e18 FracX and adds liq before the owner's first deposit using a far different and absurd ratio, they can unbalance the pool.
  4. Now, the liq deposits with reasonable ratios will be reverted because of the slip protection(if it is strict). The attacker can also front-run those deposits with more dust attacks for griefing purposes.
  5. if the LPs deposit with a bigger slippage protection, the attacker can make a profit on them.

Tools Used

Recommended Mitigation Steps

Perform the first deposit to pool when it is first created, calling nftAdd() from the constructor.

@code423n4 code423n4 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Dec 19, 2022
code423n4 added a commit that referenced this issue Dec 19, 2022
@c4-judge
Copy link
Contributor

berndartmueller marked the issue as duplicate of #442

C4-Staff added a commit that referenced this issue Jan 6, 2023
@c4-judge c4-judge removed the 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value label Jan 10, 2023
@c4-judge
Copy link
Contributor

berndartmueller changed the severity to 3 (High Risk)

@c4-judge c4-judge added 3 (High Risk) Assets can be stolen/lost/compromised directly upgraded by judge Original issue severity upgraded from QA/Gas by judge labels Jan 10, 2023
@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 upgraded by judge Original issue severity upgraded from QA/Gas by judge
Projects
None yet
Development

No branches or pull requests

2 participants