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

0 stars 0 forks source link

Glamorous Violet Chameleon - Querying for the swap amounts does not account for the swap fee when the quote and base token are the same in the from Pool #27

Open sherlock-admin2 opened 15 hours ago

sherlock-admin2 commented 15 hours ago

Glamorous Violet Chameleon

Medium

Querying for the swap amounts does not account for the swap fee when the quote and base token are the same in the from Pool

Summary

The query instruction will not check if the quote pool has enough funds to handle the swap fee when the from-pool has the same base and quote token.

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

However, the swap instruction always checks that the quote pool has enough funds with sub_reserve(). This discrepancy in behavior between query() and swap() will cause some swaps to fail when using a from-pool that has only 1 token. query() will be used by swappers as a reference for the expected output amount given an input amount.

Root Cause

In query.rs:118-123, the check if the reserve has enough funds for the swap fee is not done when the from-pool has the same base and quote token.

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

Although this same check exists in swap(), the quote pool is still implicitly checked if it has enough funds for the swap fee through sub_reserve().

ref: sub_reserve()

  pub fn sub_reserve(&mut self, amount: u128) -> Result<()> {
      if amount > self.reserve {
          return Err(ErrorCode::ReserveNotEnough.into());
      }

      self.reserve -= amount;

      Ok(())
  }

Internal pre-conditions

  1. The quote pool does not have enough reserves for a swap fee.

External pre-conditions

None

Attack Path

  1. A swapper gets the expected out amount given an input amount by calling the query() instruction off-chain before swapping.
  2. The swapper executes the swap() with the same input amount but it fails because there aren't enough reserves in the quote pool.

Impact

When the quote pool has low reserves, some swaps will unexpectedly fail due to the difference in behavior of query() and swap().

PoC

No response

Mitigation

Consider modifying the query() to always check whether the reserve and token vault have enough funds to handle the swap fee. This makes the behavior match the swap() instruction.

toprince commented 12 hours ago

Not valid for this one.

If from token is quote token. swap fee is x% of from amount. So you know what happened.