fee calculation isn't correct

[hats-bug-reporter[bot]]

Github username: -- Twitter username: -- Submission hash (on-chain): 0xe3257595f501d3b1964e09e0def872e6caef397139449329b99075343e86fe71 Severity: high

Description: Description\ Since fee is charged in swap_exact_in swap_exact_out swap_received and add_liquidity remove_liquidity_by_amounts, I will take lib._swap_exact_in as an example.

In lib._swap_exact_in, math::rated_swap_to is called in lib.rs#L444-L452, and math::rated_swap_to is defined in mod.rs#L240-L264. Then mod.swap_to is called in mod.rs#L252-L259

According to mod.swap_to, we can see that the amount of fee is calculated based on "token_out" amount, and the amount of output token is deducted the fee from mod.rs#L234-L237

209 fn swap_to(
210     token_in_id: usize,
211     token_in_amount: u128,
212     token_out_id: usize,
213     current_reserves: &Vec<u128>,
214     fees: &Fees,
215     amp_coef: u128,
216 ) -> Result<(u128, u128), MathError> {
...
228     let dy = current_reserves[token_out_id]
229         .checked_sub(y)
230         .ok_or(MathError::SubUnderflow(7))?
231         .checked_sub(1)
232         .ok_or(MathError::SubUnderflow(8))?;
233     // fees are applied to "token_out" amount
234     let fee = fees.trade_fee_from_gross(dy)?; <<<--- Here the amount of fee is applied to "token_out" amount
235     let amount_swapped = dy.checked_sub(fee).ok_or(MathError::SubUnderflow(9))?; <<<--- here is the amount of output token a user can get
236 
237     Ok((amount_swapped, fee))
238 }

Then back to lib._swap_exact_in, the contract calls decrease_reserve with decrease_reserve as input in lib.rs#L461, which means there will fee amount of token_out token left in contract

And then self.mint_protocol_fee is called in lib.rs#L464

According to lib.mint_protocol_fee

In lib.rs#L378, the fee is applied to self.pool.fees.protocol_trade_fee function again, and then the lp share is calculated based on protocol_fee in lib.rs#L387-L396

 376         fn mint_protocol_fee(&mut self, fee: u128, token_id: usize) -> Result<(), StablePoolError> {
 377             if let Some(fee_to) = self.fee_receiver() {
 378                 let protocol_fee = self.pool.fees.protocol_trade_fee(fee)?;  <<< --- here fee is used to calcate trade fee 
 379                 if protocol_fee > 0 {
 380                     let rates = self.get_scaled_rates(&self.pool.token_rates)?;
 381                     let mut protocol_deposit_amounts = vec![0u128; self.pool.tokens.len()];
 382                     protocol_deposit_amounts[token_id] = protocol_fee;
 383                     let mut reserves = self.pool.reserves.clone();
 384                     reserves[token_id] = reserves[token_id]
 385                         .checked_sub(protocol_fee)
 386                         .ok_or(MathError::SubUnderflow(102))?;
 387                     let (protocol_fee_lp, _) = math::rated_compute_lp_amount_for_deposit(
 388                         &rates,
 389                         &protocol_deposit_amounts, <<< --- trade fee is used here as final fee the fee receiver can get
 390                         &reserves,
 391                         self.psp22.total_supply(),
 392                         None, // no fees
 393                         self.amp_coef(),
 394                     )?;
 395                     // mint fee (shares) to protocol
 396                     let events = self.psp22.mint(fee_to, protocol_fee_lp)?;
 397                     self.emit_events(events);
 398                 }
 399             }
 400             Ok(())
 401         }

And in lib.rs#L396, protocol_fee_lp is the final lp share the receiver can get, which isn't correct.

Becase as we described in above:

1. after calling math::rated_swap_to, there be fee amount of token left in the contract
2. in self.mint_protocol_fee, only fee * self.protocol_fee / FEE_DENOM will be mint for fee receiver
3. there will be fee - fee * self.protocol_fee / FEE_DENOM should mint for fee receiver.

Attack Scenario\ Describe how the vulnerability can be exploited.

Attachments

1. Proof of Concept (PoC) File

2. Revised Code File (Optional)

[JanKuczma]

Thank you for your submission.

1. after calling math::rated_swap_to, there be fee amount of token left in the contract
2. in self.mint_protocol_fee, only fee * self.protocol_fee / FEE_DENOM will be mint for fee receiver

That's correct and intentional. The (protocol)fee_receiver gets its fee part in the form of LP token, just like the fee_receiver would make a deposit of this protocol_fee, without fee ofc. Before calculating the protocol_fee_lp, the protocol_fee is subtracted from the token_out reserve, so the calculation of protocol_fee_lp is fair.

3. there will be fee - fee * self.protocol_fee / FEE_DENOM should mint for fee receiver.

The fee_receiver will receive the protocol_fee_lp amount of the LP token. The whole fee amount will be left of the contract meaning that fee - protocol_fee will contribute to the LP token value increase.