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

2 stars 2 forks source link

0xeix - Spread is not updated for base tokens when performing base to base swap #12

Open sherlock-admin2 opened 1 month ago

sherlock-admin2 commented 1 month ago

0xeix

Medium

Spread is not updated for base tokens when performing base to base swap

Summary

Currently the function handler() is supposed to update the spread and the state afterwards of both base tokens when performing base to base swap. However, it does not do it.

Vulnerability Detail

Take a look at how state updates are made inside of handler() function:

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

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

However, in the EVM implementation of the contracts, it's done the following way:

https://github.com/woonetwork/WooPoolV2/blob/main/contracts/WooPPV2.sol#L545-558

    {
            uint64 spread = _maxUInt64(state1.spread, state2.spread) / 2;
            uint16 feeRate = _maxUInt16(tokenInfos[baseToken1].feeRate, tokenInfos[baseToken2].feeRate);

            state1.spread = spread;
            state2.spread = spread;

            uint256 newBase1Price;
            (quoteAmount, newBase1Price) = _calcQuoteAmountSellBase(baseToken1, base1Amount, state1);
            IWooracleV2_2(wooracle).postPrice(baseToken1, uint128(newBase1Price));
            // console.log('Post new base1 price:', newBase1Price, newBase1Price/1e8);

            swapFee = (quoteAmount * feeRate) / 1e5;
        }

As you can see from code snippets above, in the first case we don't take spread into account at all. This results in an incorrect spread value being used when calculating amount for both operations (selling base to quote, selling quote to base) as we take not the average spread as in the EVM version but the spread of the base token:

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

        spread: oracle.spread;

https://github.com/sherlock-audit/2024-08-woofi-solana-deployment/blob/main/WOOFi_Solana/programs/woofi/src/util/swap_math.rs#L54-58

let calc_b: u128 = ONE_E18_U128
        .checked_sub(gamma)
        .unwrap()
        .checked_sub(state.spread as u128)
        .unwrap();

https://github.com/sherlock-audit/2024-08-woofi-solana-deployment/blob/main/WOOFi_Solana/programs/woofi/src/util/swap_math.rs#L104-108

let calc_c: u128 = ONE_E18_U128
        .checked_sub(gamma)
        .unwrap()
        .checked_sub(state.spread as u128)
        .unwrap();

As the protocol states for the EVM and Solana contracts to be identical, this can be also considered as a deviation from the spec.

Impact

Incorrect spread being used may affect base amount calculation.

Code Snippet

Provided above.

Tool used

Manual Review

Recommendation

Calculate the average spread of the base token 1 and base token 2 by fetching their corresponding states from the oracle and use this spread in calculation of base amount in swap_math.

toprince commented 1 month ago

You are very careful when checking if 2 contracts are identical. Here is the main difference when implement in solana. Previously, the same logic in solana also raised when auditing the evm contract. So we choose to use this version in solana. So this is by design...