CodeHawks FirstFlight - SSSwap

Here is a sample report written during a practice run with CodeHawks Rust AMM named SSSwap

Description

The remove_liquidity() function fails to properly handle precision loss during integer division when calculating the amounts of tokens to return to the liquidity provider. This can result in a disproportionate amount of token_a and token_b returned, leading to an imbalance in the pool’s reserves. This imbalance is not checked in swap calculations which can allow an attacker to drain disproportionate amounts of tokens.

When a liquidity provider redeems LP tokens, the amounts of token_a and token_b to be returned are calculated using the formulas (lpt_to_redeem * reserve_a) / total_lp_supply and (lpt_to_redeem * reserve_b) / total_lp_supply. If lpt_to_redeem * reserve_X is less than total_lp_supply, then amount_X_to_return will be 0 due to integer truncation. Due to the ratio maintained of tokens in the liquidity pool, this scenario can occur for one token while the other returns a non-zero amount, skewing the pool’s ratio without a corresponding burn of LP tokens for the “lost” token.

This creates a “stale” or “inflated” reserve for the token that was not withdrawn, which the swap_exact_out() function then incorrectly uses for its calculations.

pub fn remove_liquidity(context: Context<ModifyLiquidity>, lpt_to_redeem: u64) -> Result<()> {

// ...

let total_lp_supply = context.accounts.lp_mint.supply;

let reserve_a = context.accounts.vault_a.amount;

let reserve_b = context.accounts.vault_b.amount;

// ...

let amount_a_to_return_u128 = (lpt_to_redeem as u128)

.checked_mul(reserve_a as u128)

.ok_or(AmmError::Overflow)?

.checked_div(total_lp_supply as u128)

.ok_or(AmmError::DivisionByZero)?;

  

let amount_a_to_return = amount_a_to_return_u128 as u64;

  

let amount_b_to_return_u128 = (lpt_to_redeem as u128)

.checked_mul(reserve_b as u128)

.ok_or(AmmError::Overflow)?

.checked_div(total_lp_supply as u128)

.ok_or(AmmError::DivisionByZero)?;

  

let amount_b_to_return = amount_b_to_return_u128 as u64;

  

// This check only prevents BOTH from being 0 leading to the imbalance.

@> if amount_a_to_return == 0 && amount_b_to_return == 0 && lpt_to_redeem > 0 {

return err!(AmmError::CalculationFailure);

}

  

// ... burn LPTs ...

  

// Conditional transfers means that while withdraw value is 0, the other remains untouched

if amount_a_to_return > 0 {

// ... transfer_checked token A ...

}

  

if amount_b_to_return > 0 {

// ... transfer_checked token B ...

}

// ...

}

Risk

Likelihood: High

• If reserve_a is much smaller than reserve_b, then even a moderately sized lpt_to_redeem can result in the smaller reserve’s calculated return amount truncating to 0, while the larger reserve’s amount remains positive.

Impact: High

• The attacker can identify that one token is artificially cheaper within the pool than its true market value due to the inflated reserve. They can then execute swap_exact_out transactions to drain the correctly priced token at a favorable rate such that liquidity providers suffer significant loss and can have their funds drained.

Proof of Concept

Assume an AMM pool state where reserve_a = 100, reserve_b = 1000, and total_lp_supply = 1500.

Step 1 - remove_liquidity

If an attacker wishes to remove_liquidity by burning lpt_to_redeem = 2, then the following happens:

amount_a_to_return = (lpt_to_redeem * reserve_a) / total_lp_supply
= (2 * 100) / 1500
= 200 / 1500
= 0.13...
= 0 (due to integer truncation)
amount_b_to_return = (lpt_to_redeem * reserve_b) / total_lp_supply
= (2 * 1000) / 1500
= 2000 / 1500
= 1

The check if amount_a_to_return == 0 && amount_b_to_return == 0 && lpt_to_redeem > 0 passes since amount_b_to_return is 1, which results in an updated state where reserve_a = 100, reserve_b = 998, and total_lp_supply = 1498. The implied value for reserve_a should actually be 99.86... rather than 100, meaning reserve_a is now inflated relative to the burned LP tokens. This step can be repeated up until the result of (lpt_to_redeem * reserve_b) / total_lp_supply is exactly 1. In the example state, if this step were repeated, one could redeem enough from reserve_b to cause a state where reserve_a = 100, reserve_b = 500, and total_lp_supply = 1000.

Step 2 - swap_exact_out

Now with the pool state of reserve_a = 100 and reserve_b = 500, a call can be made to swap_exact_out with amount_out = 10 and zero_for_one = true. The following is the arithmetic that results from the current state:

numerator = reserve_a * amount_out
= 100 * 10
= 1000
denominator = reserve_b - amount_out
= 500 - 10
= 490
amount_in_no_fee = numerator.div_floor(&denominator)
= 1000 / 490
= 2

Here, the malicious actor indicated they wanted to buy 10 of token_b. The amount calculated from the current reserves indicates the malicious actor will have to supply 2 token_a to get 10 token_b. Because of this, amount_in_final = 2 and the new pool state is reserve_a = 100 + 2 = 102 and reserve_b = 500 - 10 = 490. The attacker has now obtained 10 token_b for the price of 2 token_a, even though the pool started at a 1:10 ratio and should have intended to maintain something close to that.

Recommended Mitigation

The primary mitigation is to prevent remove_liquidity from executing if it results in a non-proportional return. This ensures that the pool’s state remains consistent with the LP tokens burned.

Diff

pub fn remove_liquidity(context: Context<ModifyLiquidity>, lpt_to_redeem: u64) -> Result<()> {
// ...
- if amount_a_to_return == 0 && amount_b_to_return == 0 && lpt_to_redeem > 0 {
+ // Prevent disproportionate returns or zero returns for non-zero redemption
+ if (amount_a_to_return == 0 || amount_b_to_return == 0) && lpt_to_redeem > 0 {
return err!(AmmError::CalculationFailure);
}
// ... burn LPTs ...
}