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

2 stars 2 forks source link

0xeix - Swap fees are only taken for the first swap #5

Open sherlock-admin4 opened 1 month ago

sherlock-admin4 commented 1 month ago

0xeix

Medium

Swap fees are only taken for the first swap

Summary

Swap fees in the handler() function are only taken for the first swap but not for the second swap which is not an expected behavior.

Vulnerability Detail

The current functionality of the handler() function that performs the swap takes fees the following way:

Let's say we have SOL / USDT and USDC (or ETH but it's not supported yet) / USDT pools and we perform a swap from SOL to USDC. The first swap is when we convert SOL to the quote token (USDT) and the second is when we swap USDT on USDC obtaining the final amount. Currently the fees are only taken on the first swap:

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

But they're not taken on the second swap:

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


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

In comparison, EVM implementation of the same functionality takes swap on both operations - when selling quote and when selling base:

https://github.com/woonetwork/WooPoolV2/blob/main/contracts/WooPPV2.sol#L453

uint256 swapFee = (quoteAmount * tokenInfos[baseToken].feeRate) / 1e5;

https://github.com/woonetwork/WooPoolV2/blob/main/contracts/WooPPV2.sol#L492

    uint256 swapFee = (quoteAmount * tokenInfos[baseToken].feeRate) / 1e5;

Impact

The swap fees are not taken on the swap from the quote token to the woopool_to.token_mint (selling quote operation):

Code Snippet

Provided above.

Tool used

Manual Review

Recommendation

Take the fee on the _to_amount after selling quote token.

toprince commented 1 month ago

Not valid for this one. It's by design here. Ln120-Ln126 indicates fee rate is the max rate of from token and to token.