search

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

0 stars 0 forks source link

Uneven Gingham Locust - Swap function does not check `woopool_quote` is indeed a quote pool #40

Open sherlock-admin3 opened 15 hours ago

sherlock-admin3 commented 15 hours ago

Uneven Gingham Locust

Medium

Swap function does not check woopool_quote is indeed a quote pool

Summary

WooFi defines a quote pool has the pool which has pool.token_mint == pool.quote_token_mint i.e pool for (Q, Q) where Q is the quote token for a set of pools.

The swap function does not check this condition for the woopool_quote account. It only checks that the woopool_quote.token_mint == woopool_from.quote_token_mint.

It is possible for a pool to exist for a pair (R, S) where R is the quote_token for some other pools X, Y, Z but for this pool R is the base token and S is the quote token.

The pool (R, S) is not the quote pool for X, Y, Z but can be used as quote pool in swaps between X, Y, Z.

As a result, an user can call swap with (R, S) as the woopool_quote for a swap between (X, R), (Y, R) pools and when the swap function deducts the fee it will be deducted from a base pool instead of the quote pool (R, R).

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

Root Cause

Missing check in the swap function for the woopool_quote account:

woopool_quote.token_mint == woopool_quote.quote_token_mint.

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

Internal pre-conditions

There exists a pool which has a quote token as base token with a different quote token.

For example:

USDC is the quote token and there are pools for (SOL, USDC), (USDT, USDC), (USDC, USDC). SOL and USDT are base tokens. (USDC, USDC) is the quote pool for these pools. Fees should be deducted from the (USDC, USDC) pool when SOL is swapped to USDT.

The vulnerability requires that there exists a pool for (USDC, Q) pair. For this pool, USDC is a base token and Q is the quote token. hence is not a quote pool for the SOL, USDT pairs.

The code indicates of this possibility:

  1. The account constraints for the swap function ensures the woopool_from, woopool_to, wooracle_from, _to all have same quote_token_mint as a protection from usage of swaps between pools with different quote tokens. This shows that there could be multiple quote tokens.

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

  1. The seeds used for the rebate manager includes the quote_token_mint allowing for a different manager for each quote token

https://github.com/sherlock-audit/2024-08-woofi-solana-deployment/blob/main/WOOFi_Solana/programs/rebate_manager/src/instructions/admin/create_rebate_manager.rs#L14-L23

  1. The use of quote token mint in the seeds of woopool further indicate this possibility

External pre-conditions

No response

Attack Path

User performs the swap operation with the following parameters.

After the swap operation, the fees are deducted into a base pool (USDC, Q) instead of the quote pool (USDC, USDC) for the SOL, USDT pairs.

Impact

Users can make fees to be deducted from a different pool than the intended pool essentially reducing the reserves for the base pool.

Correct implementation of claim fees disallows claiming the fees from a base pool. In the current implementation, balance function is called before the transfer instead of after. Correcting the function would increase the impact of this issue.

PoC

No response

Mitigation

Add woopool_quote.token_mint == wooool_quote.quote_toke_mint check in the swap function.

toprince commented 12 hours ago

Need further investigation.