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

Price manipulation by sending Ether #473

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

Price manipulation by sending Ether #473

code423n4 opened this issue Dec 19, 2022 · 6 comments
Labels
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 duplicate-383 partial-50 Incomplete articulation of vulnerability; eligible for partial credit only (50%)

Comments

@code423n4
Copy link
Contributor

Lines of code

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

Vulnerability details

Price manipulation by sending Ether (not profitable although)

The function Pair.price() uses Pair._baseTokenReserves uses address(this).balance or ERC20(baseToken).balanceOf(address(this)). In case address(this).balance is used (when address(0) is passed), there can be price manipulation, since it is not possible to always control the amount of ether the contract has (someone could send ether to the contract by calling selfdestruct(addr) - increasing the price of the base token. Although, It is not profitable for the user to do that, the contract is still vulnerable to prices manipulation.

Proof of Concept

Pair.sol#L479

Recommended Mitigation Steps

One state variable could be used to track the eth balance instead. Using address(this).balance should be avoided when directly linked to price setup.

@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
@iFrostizz
Copy link

No in-scope contracts are relying on the price(). Additionally, we could send tokens to the pair in order to manipulate the price but it would be the same for an attacker to add liquidity.

@Shungy
Copy link
Member

Shungy commented Dec 21, 2022

Duplicate #506

@c4-judge
Copy link
Contributor

berndartmueller marked the issue as duplicate of #353

@berndartmueller
Copy link
Member

Applying a partial credit as the submission only focuses on ETH-dominated pairs (and rather low quality), even though the issues also exist for baseToken (contrary to the report #383)

@c4-judge
Copy link
Contributor

berndartmueller marked the issue as partial-50

@c4-judge c4-judge added the partial-50 Incomplete articulation of vulnerability; eligible for partial credit only (50%) label Jan 13, 2023
@C4-Staff
Copy link
Contributor

CloudEllie marked the issue as duplicate of #383

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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 duplicate-383 partial-50 Incomplete articulation of vulnerability; eligible for partial credit only (50%)
Projects
None yet
Development

No branches or pull requests

6 participants