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

[M04] Missing check for input=output in stableswap math #88

Closed
c4-bot-1 opened this issue Feb 27, 2024 · 6 comments
Closed

[M04] Missing check for input=output in stableswap math #88

c4-bot-1 opened this issue Feb 27, 2024 · 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 insufficient quality report This report is not of sufficient quality unsatisfactory does not satisfy C4 submission criteria; not eligible for awards

Comments

@c4-bot-1
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2024-02-hydradx/blob/603187123a20e0cb8a7ea85c6a6d718429caad8d/HydraDX-node/pallets/stableswap/src/lib.rs#L787-L842

Vulnerability details

Impact

In curve protocol, there is a check that idx_in != idx_out here. This is there to prevent swapping of the same token with itself.

The stableswap protocol however is missing this check. This can potentially allow users to swap the same token for itself. This can break the math of the stableswap protocol, and potentially lead to wrong balances.

This check is not present in the buy function, and in the calculate_in_amount function, the only checks are on the individual tokens.

let index_in = pool.find_asset(asset_in).ok_or(Error::<T>::AssetNotInPool)?;
let index_out = pool.find_asset(asset_out).ok_or(Error::<T>::AssetNotInPool)?;

Proof of Concept

The check is missing in the code.

Tools Used

Manual Review

Recommended Mitigation Steps

Add in a check to ensure idx_in != idx_out

Assessed type

Invalid Validation

@c4-bot-1 c4-bot-1 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 Feb 27, 2024
c4-bot-1 added a commit that referenced this issue Feb 27, 2024
@c4-pre-sort
Copy link

0xRobocop marked the issue as insufficient quality report

@c4-pre-sort c4-pre-sort added the insufficient quality report This report is not of sufficient quality label Mar 3, 2024
@c4-pre-sort
Copy link

0xRobocop marked the issue as duplicate of #58

@0xRobocop
Copy link

Duplicated to #58, but this report does not describe any impact or attack, consider to either invalidate or partial.

@c4-judge
Copy link

c4-judge commented Mar 7, 2024

OpenCoreCH marked the issue as unsatisfactory:
Insufficient proof

@c4-judge c4-judge added the unsatisfactory does not satisfy C4 submission criteria; not eligible for awards label Mar 7, 2024
@OpenCoreCH
Copy link

Invalidating this issue because it does not describe any impact and just speculates about potential impacts. The issue itself would not have allowed the sponsor to see that this is a problem, whereas the other two wardens described the impacts in detail.

@c4-judge
Copy link

c4-judge commented Mar 9, 2024

OpenCoreCH marked the issue as not a duplicate

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 insufficient quality report This report is not of sufficient quality unsatisfactory does not satisfy C4 submission criteria; not eligible for awards
Projects
None yet
Development

No branches or pull requests

5 participants