Root + Impact
Description
In the swap operations swap_exact_in() and swap_exact_out, there is a missing check to prevent lp_fees from being 0. Considering there is no limit on how low a swap amount can be, a user can just constantly request small swaps until their intended amount of swaps is completed and fully avoid paying any fees to the network. This is because the code treats the lp_fee as a u64 which means that if it is anything less than 1 it will get truncated to 0 and the transaction will not cost anything to perform.
pub fn swap_exact_out(context: Context<SwapContext>, amount_out: u64, max_in: u64, zero_for_one: bool) -> Result<()> {
if zero_for_one {
let fee_numerator_u128 = (amount_in_no_fee as u128)
.checked_mul(3)
.ok_or(AmmError::Overflow)?;
@> let lp_fees: u64 = fee_numerator_u128.div_floor(&1000) as u64;
let amount_in_final = (amount_in_no_fee as u128)
.checked_add(lp_fees as u128)
.ok_or(AmmError::Overflow)? as u64;
require!(amount_in_final > 0 || (amount_in_final == 0 && amount_out == 0), AmmError::CalculationFailure);
require!(amount_in_final <= max_in, AmmError::Slippage);
}
Risk
Likelihood: High
Impact:
Proof of Concept
The code below can be run with cargo run poc.rs and should write to the console the value of lp_fees = 0.
fn swap(amount_in: u64) -> Result<(), String>{
let reserve_a: u64 = 100;
let reserve_b: u64 = 500;
let numerator: u128 = (reserve_b as u128).checked_mul(amount_in as u128).ok_or("Overflow".to_string())?;
let denominator: u128 = (reserve_a as u128).checked_add(amount_in as u128).ok_or("Overflow".to_string())?;
if denominator == 0 {
return Err("DivisionByZero".to_string());
}
println!("Numerator: {} and Denominator {} and Casted Denominator {}", numerator, denominator, denominator as u64);
let mut amount_out: u64 = numerator.checked_div(denominator).ok_or("CalcError".to_string()).unwrap() as u64;
println!("Amount Out: {amount_out}");
let lp_fees = (amount_out as u128 * 3).checked_div(1000).ok_or("CalcError".to_string()).unwrap() as u64;
println!("lp_fees = {lp_fees}");
amount_out = amount_out - lp_fees;
if amount_out == 0 || amount_out < 1 {
return Err("Slippage".to_string());
}
Ok(())
}
fn main() {
let amount_in: u64 = 10;
let _swap = swap(amount_in);
println!("{:?}", _swap)
}
Recommended Mitigation
Add a check to guarantee at least 1 token be charged for a transaction.
pub fn swap_exact_out(context: Context<SwapContext>, amount_out: u64, max_in: u64, zero_for_one: bool) -> Result<()> {
// ...
if zero_for_one {
// ...
let fee_numerator_u128 = (amount_in_no_fee as u128)
.checked_mul(3)
.ok_or(AmmError::Overflow)?;
let lp_fees: u64 = fee_numerator_u128.div_floor(&1000) as u64;
+ require!(lp_fees > 0), AmmError::CalculationFailure);
let amount_in_final = (amount_in_no_fee as u128)
.checked_add(lp_fees as u128)
.ok_or(AmmError::Overflow)? as u64;
require!(amount_in_final > 0 || (amount_in_final == 0 && amount_out == 0), AmmError::CalculationFailure);
require!(amount_in_final <= max_in, AmmError::Slippage);
// ... Transfer Tokens ...
}
