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

2 stars 2 forks source link

0xeix - Fees are inconsistently charged if the price direction quote -> base and base -> quote #35

Open sherlock-admin4 opened 1 month ago

sherlock-admin4 commented 1 month ago

0xeix

Medium

Fees are inconsistently charged if the price direction quote -> base and base -> quote

Summary

The protocol charges fees at the moment on every swap. The problem is that depending on the direction of the swap this amount may differ resulting in a loss of potential slippage funds by the protocol (or taking excessive funds making some users paying more).

Vulnerability Detail

At the moment the fees are taken from the quote_amount:

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

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

The problem is that in one case it's taken from the initial amount and in the other after performing the first swap:

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

  quote_amount = _quote_amount;

Here the fees are taking after selling base token to the quote token.

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

 let mut quote_amount = from_amount;

And here it's taken from the initial from_amount meaning it has a certain value every time. So if the direction of the swap is quote -> base (selling the quote token and getting the base token) the first if-statement will be skipped and we'll take the fees from the initial from_amount. In the second case, the swap can be subjected to MEV attacks and therefore every time the protocol may lose some fees due to the slippage.

Impact

The protocol will lose fee money due to the inconsistent fee charge.

Code Snippet

Tool used

Manual Review

Recommendation

Take fees on the initial amount to avoid slippage impact.

toprince commented 1 month ago
  1. quote->base and base->quote are 2 different swaps, so the fee may have difference
  2. Need more real calc to see if will lose fee in such case. Please input more.