sherlock-audit / 2024-08-woofi-solana-deployment-judging

2 stars 2 forks source link

LZ_security - There is an issue with the denomination of token and calculation formula of the swap fee in the swap() function. #56

Open sherlock-admin4 opened 2 months ago

sherlock-admin4 commented 2 months ago

LZ_security

Medium

There is an issue with the denomination of token and calculation formula of the swap fee in the swap() function.

Summary

There is an issue with the denomination of token and calculation formula of the swap fee in the swap() function.

Vulnerability Detail

pub fn handler(ctx: Context<Swap>, from_amount: u128, min_to_amount: u128) -> Result<()> {
    let price_update_from = &mut ctx.accounts.price_update_from;
    let price_update_to = &mut ctx.accounts.price_update_to;
    let quote_price_update = &mut ctx.accounts.quote_price_update;

    let token_owner_account_from = &ctx.accounts.token_owner_account_from;
    require!(
        token_owner_account_from.amount as u128 >= from_amount,
        ErrorCode::NotEnoughBalance
    );

    let token_vault_from = &ctx.accounts.token_vault_from;
    let token_owner_account_to = &ctx.accounts.token_owner_account_to;
    let token_vault_to = &ctx.accounts.token_vault_to;
    let quote_token_vault = &ctx.accounts.quote_token_vault;
    let woopool_quote = &mut ctx.accounts.woopool_quote;

    let wooracle_from = &mut ctx.accounts.wooracle_from;
    let woopool_from = &mut ctx.accounts.woopool_from;

    let wooracle_to = &mut ctx.accounts.wooracle_to;
    let woopool_to = &mut ctx.accounts.woopool_to;
    let rebate_to = &ctx.accounts.rebate_to;

    let fee_rate: u16 = if woopool_from.token_mint == woopool_from.quote_token_mint {
        woopool_to.fee_rate
    } else if woopool_to.token_mint == woopool_to.quote_token_mint {
        woopool_from.fee_rate
    } else {
@>>        max(woopool_from.fee_rate, woopool_to.fee_rate)
    };

    let decimals_from = Decimals::new(
        wooracle_from.price_decimals as u32,
        wooracle_from.quote_decimals as u32,
        woopool_from.base_decimals as u32,
    );

    let mut quote_amount = from_amount;
    if woopool_from.token_mint != woopool_from.quote_token_mint {
        let state_from =
            get_price::get_state_impl(wooracle_from, price_update_from, quote_price_update)?;

        let (_quote_amount, new_base_price) = swap_math::calc_quote_amount_sell_base(
            from_amount,
            woopool_from,
            &decimals_from,
            &state_from,
        )?;

        wooracle_from.post_price(new_base_price)?;

        quote_amount = _quote_amount;
    }

    let swap_fee = checked_mul_div_round_up(quote_amount, fee_rate as u128, ONE_E5_U128)?;
    quote_amount = quote_amount.checked_sub(swap_fee).unwrap();

    if woopool_from.token_mint != woopool_from.quote_token_mint {
        require!(
@>>            woopool_quote.reserve >= swap_fee && quote_token_vault.amount as u128 >= swap_fee,
            ErrorCode::NotEnoughOut
        );
    }

    let decimals_to = Decimals::new(
        wooracle_to.price_decimals as u32,
        wooracle_to.quote_decimals as u32,
        woopool_to.base_decimals as u32,
    );

    let mut to_amount = quote_amount;
    if woopool_to.token_mint != woopool_to.quote_token_mint {
        let state_to = get_price::get_state_impl(wooracle_to, price_update_to, quote_price_update)?;

        let (_to_amount, to_new_price) = swap_math::calc_base_amount_sell_quote(
            quote_amount,
            woopool_to,
            &decimals_to,
            &state_to,
        )?;
        wooracle_to.post_price(to_new_price)?;

        to_amount = _to_amount;
    }

    require!(
        woopool_to.reserve >= to_amount && token_vault_to.amount as u128 >= to_amount,
        ErrorCode::NotEnoughOut
    );

    require!(to_amount >= min_to_amount, ErrorCode::AmountOutBelowMinimum);

    woopool_from.add_reserve(from_amount).unwrap();
    woopool_to.sub_reserve(to_amount).unwrap();

    // record fee into account
    woopool_quote.sub_reserve(swap_fee).unwrap();
    woopool_quote.add_unclaimed_fee(swap_fee).unwrap();

    transfer_from_owner_to_vault(
        &ctx.accounts.payer,
        token_owner_account_from,
        token_vault_from,
        &ctx.accounts.token_program,
        from_amount as u64,
    )?;

    transfer_from_vault_to_owner(
        woopool_to,
        token_vault_to,
        token_owner_account_to,
        &ctx.accounts.token_program,
        to_amount as u64,
    )?;

    emit!(SwapEvent {
        sender: ctx.accounts.payer.key(),
        from_token_mint: woopool_from.token_mint,
        to_token_mint: woopool_to.token_mint,
        from_amount,
        to_amount,
        from_account: token_owner_account_from.key(),
        to_account: token_owner_account_to.key(),
        rebate_to: rebate_to.key(),
        swap_vol: quote_amount + swap_fee,
        swap_fee,
    });

    Ok(())
}

1. Payment Currency

In the cryptocurrency field, swap fees are typically charged in the currency you are receiving. For example, if you are swapping WETH for USDT, the fee is charged in USDT; if you are swapping USDT for WETH, the fee is charged in WETH; and if you are swapping WETH for WBTC, the fee is charged in WBTC. This leads to two issues:

(1). Fee Value Discrepancy: For example, if the fee is 0.01 WETH and the current price is $2000, but later the price increases to $3000, the 80% of the fee refunded to the user should be based on the updated price, i.e., 0.01 80% $3000. (2). Pool Reserve Reversion: if a user is using WBTC to swap for WETH in a from_pool (base_token: WBTC, quote_token: USDT) and to_pool (base_token: WETH, quote_token: USDT), and if the reserve in the quote_pool (base_token: USDT, quote_token: USDT) is less than the swap fee, the transaction will revert. This is also unreasonable because the user does not need USDT and is paying in WBTC.

2. Fee Rate

In the cryptocurrency field, swap fees are typically calculated based on the fee rate of the pool corresponding to the currency you are receiving. For example, if you are swapping WETH for USDT, the fee rate should be taken from the USDT pool; if you are swapping USDT for WETH, the fee rate should be taken from the WETH pool; and if you are swapping WETH for WBTC, the fee rate should be taken from the WBTC pool.

However, in this protocol, for example, when swapping WETH for WBTC, the fee rate is calculated as:

max(woopool_from.fee_rate, woopool_to.fee_rate)

If the transaction were to follow the real process of first swapping WETH to USDT (usdt_pool) and then USDT to WBTC (btc_pool), the fee rate should be:

woopool_from.fee_rate + woopool_to.fee_rate

Impact

This results in incorrect fee collection and potential DoS (Denial of Service) issues.

Code Snippet

https://github.com/sherlock-audit/2024-08-woofi-solana-deployment/blob/main/WOOFi_Solana/programs/woofi/src/instructions/swap.rs#L96

Tool used

Manual Review

Recommendation

Modify the protocol so that the fee is paid in the received token and is calculated based on the fee rate of the pool being swapped.

toprince commented 2 months ago

Not valid. Swap fee is in quote token.