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

Incorrect token-in transfer amount in swap_2_internal_erc20 #16

Closed
howlbot-integration bot opened this issue Nov 4, 2024 · 1 comment
Closed
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working duplicate-5 🤖_02_group AI based duplicate group recommendation satisfactory satisfies C4 submission criteria; eligible for awards sufficient quality report This report is of sufficient quality

Comments

@howlbot-integration
Copy link

Lines of code

https://github.com/code-423n4/2024-10-superposition/blob/main/pkg/seawater/src/lib.rs#L286-L297

Vulnerability details

Proof of Concept

In the swap_2_internal function, if the first pool experiences a liquidity depletion, it could result in amount_in being less than original_amount. In this case, the contract takes amount_in tokens from the user and returns original_amount - amount_in tokens back to the user. However, the correct calculation should take original_amount tokens from the user first. The pool loses original_amount - amount_in tokens in such cases.

let (
    original_amount,
    amount_in,
    amount_out,
    _interim_usdc_out,
    _final_tick_in,
    _final_tick_out,
) = Self::swap_2_internal(pools, from, to, amount, min_out)?;

#[cfg(feature = "testing-dbg")]
dbg!((
    "swap 2 internal erc20 after internal",
    amount_in.to_string(),
    amount_out.to_string()
));

// transfer tokens
erc20::take(from, amount_in, permit2)?;
erc20::transfer_to_sender(to, amount_out)?;

if original_amount > amount_in {
    erc20::transfer_to_sender(
        to,
        original_amount
            .checked_sub(amount_in)
            .ok_or(Error::TransferToSenderSub)?,
    )?;
}

Recommended Mitigation Steps

-        erc20::take(from, amount_in, permit2)?;
+        erc20::take(from, original_amount, permit2)?;

Assessed type

Math

@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-5 sufficient quality report This report is of sufficient quality labels Nov 4, 2024
howlbot-integration bot added a commit that referenced this issue Nov 4, 2024
@c4-judge c4-judge added the satisfactory satisfies C4 submission criteria; eligible for awards label Nov 13, 2024
@c4-judge
Copy link

alex-ppg marked the issue as satisfactory

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-5 🤖_02_group AI based duplicate group recommendation satisfactory satisfies C4 submission criteria; eligible for awards sufficient quality report This report is of sufficient quality
Projects
None yet
Development

No branches or pull requests

1 participant