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

2 stars 2 forks source link

LZ_security - Swaps can happen without changing the price for the next trade due to gamma = 0 #70

Open sherlock-admin3 opened 1 month ago

sherlock-admin3 commented 1 month ago

LZ_security

Medium

Swaps can happen without changing the price for the next trade due to gamma = 0

Summary

When a swap happens in WoofiPool the price is updated accordingly respect to such value "gamma". However, there are some cases where the swap results to a "gamma" value of "0" which will not change the new price for the next trade.

Vulnerability Detail

This is how the quote token received and new price is calculated when given amount of base tokens are sold to the pool:

pub fn calc_quote_amount_sell_base(
    base_amount: u128,
    woopool: &Account<'_, WooPool>,
    decimals: &Decimals,
    state: &GetStateResult,
) -> Result<(u128, u128)> {
    require!(state.feasible_out, ErrorCode::WooOracleNotFeasible);

    require!(state.price_out > 0, ErrorCode::WooOraclePriceNotValid);

    //let notionalSwap : u128 = (base_amount * state.price_out * decimals.quote_dec) / decimals.base_dec / decimals.price_dec;
    let notion_calc_a: u128 =
        checked_mul_div(base_amount, state.price_out, decimals.price_dec as u128)?;
    let notional_swap: u128 = checked_mul_div(
        notion_calc_a,
        decimals.quote_dec as u128,
        decimals.base_dec as u128,
    )?;

    require!(
        notional_swap <= woopool.max_notional_swap,
        ErrorCode::WooPoolExceedMaxNotionalValue
    );

    // gamma = k * price * base_amount; and decimal 18
    let gamma_calc_a: u128 =
        checked_mul_div(base_amount, state.price_out, decimals.price_dec as u128)?;
@>>    let gamma: u128 =
        checked_mul_div(gamma_calc_a, state.coeff as u128, decimals.base_dec as u128)?;
    require!(gamma <= woopool.max_gamma, ErrorCode::WooPoolExceedMaxGamma);

    // Formula: quoteAmount = baseAmount * oracle.price * (1 - oracle.k * baseAmount * oracle.price - oracle.spread)
    // quoteAmount =
    // (((baseAmount * state.price * decs.quoteDec) / decs.priceDec) *
    //     (uint256(1e18) - gamma - state.spread)) /
    // 1e18 /
    // decs.baseDec;
    // ====>
    // quoteAmount =
    // (((baseAmount * state.price / decs.priceDec) * decs.quoteDec) * (uint256(1e18) - gamma - state.spread)) /
    // 1e18 /
    // decs.baseDec;
    // ====>
    // a = (baseAmount * state.price / decs.priceDec)
    // b = (uint256(1e18) - gamma - state.spread)
    // quoteAmount = ((a * decs.quoteDec) * b) / 1e18 / decs.baseDec;
    //             = ((a * b) / 1e18) * decs.quoteDec / decs.baseDec

    let calc_a: u128 = checked_mul_div(base_amount, state.price_out, decimals.price_dec as u128)?;
    let calc_b: u128 = ONE_E18_U128
        .checked_sub(gamma)
        .unwrap()
        .checked_sub(state.spread as u128)
        .unwrap();
    let calc_c = checked_mul_div(calc_a, calc_b, ONE_E18_U128)?;
    let quote_amount = checked_mul_div(
        calc_c,
        decimals.quote_dec as u128,
        decimals.base_dec as u128,
    )?;

    // newPrice = oracle.price * (1 - k * oracle.price * baseAmount)
    let new_price: u128 = checked_mul_div(
        ONE_E18_U128.checked_sub(gamma).unwrap(),
        state.price_out,
        ONE_E18_U128,
    )?;

    Ok((quote_amount, new_price))
}

Now, let's assume: USDC is quoteToken, 6 decimals SOL is baseToken which has a price of 146 USDC, 9 decimals coefficient = 100 spread = 200 baseAmount (amount of SOL are sold) = 40000;

first calculate the gamma: (baseAmount state.price state.coeff) / decs.priceDec / decs.baseDec; = 40000 146.00 1e8 * 100 / 1e8 / 1e9 = 0 due to round down

let's calculate the quoteAmount will be received: quoteAmount = (((baseAmount state.price decs.quoteDec) / decs.priceDec) (uint256(1e18) - gamma - state.spread)) / 1e18 / decs.baseDec; (40000 146.00 1e8 1e6 / 1e8) * (1e18 - 0 - 200) / 1e18 / 1e9 = 5840 which is not "0".

let's calculate the new price: newPrice = ((uint256(1e18) - gamma) state.price) / 1e18; = (1e18 - 0) 146.00 1e8 / 1e18 = 146.00 1e8 which is the same price, no price changes!

That would also means if the "gamma" is "0", then this is the best possible swap outcome. If a user does this in a for loop multiple times in a cheap network, user can trade significant amount of tokens without changing the price.

Coded PoC (values are the same as in the above textual scenario):

it("swap_from_sol_to_usdc2", async ()=> {
      for(let i =0 ;i< 3 ; i++){
      // let fromAmount = 2 * LAMPORTS_PER_SOL;
      let fromAmount = 40000;

      const solTokenAccount = payerSolTokenAccount;
      const usdcTokenAccount = payerUsdcTokenAccount;
      console.log("fromWallet PublicKey:" + fromWallet.publicKey);
      console.log('solWalletTokenAccount:' + solTokenAccount);
      console.log('usdcWalletTokenAccount:' + usdcTokenAccount);    

      // increase from pool liquidity
      const transferTranscation = new Transaction().add(
        // transfer SOL to from wallet
        SystemProgram.transfer({
          fromPubkey: payerWallet.publicKey,
          toPubkey: fromWallet.publicKey,
          lamports: fromAmount,
        }),
        // trasnfer SOL to WSOL into ata account
        SystemProgram.transfer({
          fromPubkey: fromWallet.publicKey,
          toPubkey: solTokenAccount,
          lamports: fromAmount,
        }),
        // sync wrapped SOL balance
        token.createSyncNativeInstruction(solTokenAccount)
      );

      await provider.sendAndConfirm(transferTranscation, signers, { commitment: "confirmed" });

      const initBalance = await provider.connection.getBalance(fromWallet.publicKey);
      console.log("fromWallet Balance:" + initBalance);
      const tokenBalance = await provider.connection.getTokenAccountBalance(solTokenAccount);
      console.log("fromTokenAccount amount:" + tokenBalance.value.amount);
      console.log("fromTokenAccount decimals:" + tokenBalance.value.decimals);

      const fromPoolParams = await poolUtils.generatePoolParams(solTokenMint, usdcTokenMint, solFeedAccount, solPriceUpdate);
      const toPoolParams = await poolUtils.generatePoolParams(usdcTokenMint, usdcTokenMint, usdcFeedAccount, usdcPriceUpdate);
      const quotePoolParams = await poolUtils.generatePoolParams(usdcTokenMint, usdcTokenMint, usdcFeedAccount, usdcPriceUpdate);
      const [fromPrice, fromFeasible] = await poolUtils.getOraclePriceResult(fromPoolParams.wooconfig, fromPoolParams.wooracle, solPriceUpdate, usdcPriceUpdate);
      console.log(`price - ${fromPrice}`);
      console.log(`feasible - ${fromFeasible}`);

      const [toPrice, toFeasible] = await poolUtils.getOraclePriceResult(toPoolParams.wooconfig, toPoolParams.wooracle, usdcPriceUpdate, usdcPriceUpdate);
      console.log(`price - ${toPrice}`);
      console.log(`feasible - ${toFeasible}`);

      const tx = await program
        .methods
        .tryQuery(new BN(fromAmount))
        .accounts({
          wooconfig: fromPoolParams.wooconfig,
          wooracleFrom: fromPoolParams.wooracle,
          woopoolFrom: fromPoolParams.woopool,
          priceUpdateFrom: solPriceUpdate,
          wooracleTo: toPoolParams.wooracle,
          woopoolTo: toPoolParams.woopool,
          priceUpdateTo: usdcPriceUpdate,
          quotePriceUpdate: usdcPriceUpdate,
        })
        .rpc(confirmOptionsRetryTres);

      let t = await provider.connection.getTransaction(tx, {
        commitment: "confirmed",
      })

      const [key, data, buffer] = poolUtils.getReturnLog(t);
      const reader = new borsh.BinaryReader(buffer);
      const toAmount = reader.readU128().toNumber();
      const swapFee = reader.readU128().toNumber();
      console.log('toAmount:' + toAmount);
      console.log('swapFee:' + swapFee);

      await program
        .methods
        .swap(new BN(fromAmount), new BN(0))
        .accounts({
          wooconfig: fromPoolParams.wooconfig,
          tokenProgram: token.TOKEN_PROGRAM_ID,
          payer: fromWallet.publicKey,  // is the user want to do swap
          wooracleFrom: fromPoolParams.wooracle,
          woopoolFrom: fromPoolParams.woopool,
          tokenOwnerAccountFrom: solTokenAccount,
          tokenVaultFrom: fromPoolParams.tokenVault,
          priceUpdateFrom: solPriceUpdate,
          wooracleTo: toPoolParams.wooracle,
          woopoolTo: toPoolParams.woopool,
          tokenOwnerAccountTo: usdcTokenAccount,
          tokenVaultTo: toPoolParams.tokenVault,
          priceUpdateTo: usdcPriceUpdate,
          woopoolQuote: quotePoolParams.woopool,
          quotePriceUpdate: usdcPriceUpdate,
          quoteTokenVault: quotePoolParams.tokenVault,
          rebateTo: fromWallet.publicKey,
        })
        .signers([fromWallet])
        .rpc(confirmOptionsRetryTres);

        const toTokenAccountBalance = await provider.connection.getTokenAccountBalance(usdcTokenAccount);
        // console.log("toTokenAccount amount:" + toTokenAccountBalance.value.amount);
        console.log("toTokenAccount decimals:" + toTokenAccountBalance.value.decimals);
        console.log("");
      }
    });

Impact

As by design, the price should change after every trade irrelevant of the amount that is being traded. Also, in a cheap network the attack can be quite realistic. Hence, I'll label this as medium.

Code Snippet

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

Tool used

Manual Review

Recommendation

if the "gamma" is "0", then revert.

toprince commented 1 month ago

Not valid. gamma == 0 is okay in WooPP swap. We ended up with accepting gamma = 0 , when the swap volume is pretty low (which is legit).