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.

Sign up for GitHub

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

Jump to bottom

Price manipulation in coinswap::pool #26

Open
howlbot-integration bot opened this issue Jun 21, 2024 · 4 comments
Open

Price manipulation in coinswap::pool #26

howlbot-integration bot opened this issue Jun 21, 2024 · 4 comments
Labels
bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue duplicate-13 grade-b Q-05 QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax 🤖_02_group AI based duplicate group recommendation sufficient quality report This report is of sufficient quality

Comments

@howlbot-integration
Copy link

Lines of code

https://github.com/code-423n4/2024-05-canto/blob/d1d51b2293d4689f467b8b1c82bba84f8f7ea008/canto-main/x/coinswap/keeper/keeper.go#L253-L320

Vulnerability details

Description

In the protocol, the addLiquidity function includes a guard to prevent price manipulation.

However, the removeLiquidity function lacks this protection. It only restricts the amount of tokenWithdrawCoin without considering the remaining liquidity in the pool.

An attacker can exploit this by adding liquidity and then removing most of it, leaving a small amount of liquidity in the pool, and donating tokens to the pool to manipulate the price of the token pair.

Impact

Price manipulation

Recommended Mitigation

The remaining liquidity in the pool should be checked when removing liquidity.

Assessed type

Other
@howlbot-integration howlbot-integration bot added 3 (High Risk) Assets can be stolen/lost/compromised directly 🤖_02_group AI based duplicate group recommendation bug Something isn't working duplicate-7 sufficient quality report This report is of sufficient quality labels Jun 21, 2024
howlbot-integration bot added a commit that referenced this issue Jun 21, 2024
@howlbot-integration
0xSergeantPepper issue #26
c217653
@3docSec 3docSec mentioned this issue Jun 27, 2024
Closed
@c4-judge
Copy link

3docSec marked the issue as not a duplicate

@c4-judge c4-judge reopened this Jun 27, 2024
@c4-judge
Copy link

3docSec marked the issue as duplicate of #13

@3docSec 3docSec mentioned this issue Jun 27, 2024
Open
@c4-judge
Copy link

3docSec changed the severity to QA (Quality Assurance)

@c4-judge c4-judge added downgraded by judge Judge downgraded the risk level of this issue QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax grade-b and removed 3 (High Risk) Assets can be stolen/lost/compromised directly labels Jun 27, 2024
@c4-judge
Copy link

3docSec marked the issue as grade-b

@C4-Staff C4-Staff reopened this Jul 9, 2024
@C4-Staff C4-Staff added the Q-05 label Jul 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue duplicate-13 grade-b Q-05 QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax 🤖_02_group AI based duplicate group recommendation sufficient quality report This report is of sufficient quality
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@c4-judge @C4-Staff