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

2 stars 2 forks source link

0xeix - Swap fees are not correctly handled as they are substracted from the woopool_quote reserves #4

Open sherlock-admin3 opened 1 month ago

sherlock-admin3 commented 1 month ago

0xeix

Medium

Swap fees are not correctly handled as they are substracted from the woopool_quote reserves

Summary

The handler() function inside of swap.rs instruction makes incorrect assumptions about swap fees by firstly comparing them with reserves and then substracting from the reserves.

Vulnerability Detail

First, swap_fees are calculated and substracted from the quote_amount:

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

 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();

Then the protocol makes the following check:

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

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
        );
    }

The check makes sure that the reserve in woopool_quote and amount in the quote_token_vault are greater than swap_fee. This is incorrect as the fees should not be compared to the balance and reserves as they are not part of the swap and just the commission that's taken upon the swap. It's then substracted from the woopool_quote reserves:

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

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

Impact

Such checking can block receiving swap_fees if the quote_token_vault.amount is smaller than the fees, for example.

Code Snippet

Provided above.

Tool used

Manual Review

Recommendation

Remove the check for reserves and amount of the quote token vault that are compared with the swap fees.

toprince commented 1 month ago

Not valid for this one. Need check here, otherwise may not receive swap fee.