Phoenix-Protocol-Group / phoenix-contracts

Source code of the smart contracts of the Phoenix DeFi hub DEX protocol
GNU General Public License v3.0
10 stars 6 forks source link

[V-PHX-VUL-016] Invalid value returned by total_comission_amount. #216

Closed gangov closed 7 months ago

gangov commented 7 months ago

file: contracts/multihop/src/contract.rs location: simulate_swap, simulate_reverse_swap

The simulate_swap function in multihop allows a user to simulate a chain of swaps. For example, a user can swap token1 for token2 and then swap token2 for token3, allowing the user to go from an amount of token1 to an amount of token3.

Inside simulate_swap we can found the following:

operations.iter().for_each(|op| {
  let liquidity_pool_addr: Address = factory_client.query_for_pool_by_token_pair(&op.clone().offer_asset, &op.ask_asset.clone());
  let lp_client = lp_contract::Client::new(&env, &liquidity_pool_addr);
  let simulate_swap = lp_client.simulate_swap(&op.offer_asset, & next_offer_amount);
  simulate_swap_response.total_commission_amount += simulate_swap.commission_amount;
  simulate_swap_response.ask_amount = simulate_swap.ask_amount;
  simulate_swap_response
  .spread_amount 
  .push_back(simulate_swap.spread_amount);

  next_offer_amount = simulate_swap.ask_amount;
});

Code snippet from the simulate_swap function. It is a for loop for each swap operation.

The above code simulates every swap operation, and uses the results of the simulation (e.g., the returned ask_amount) as the input for the next swap operation. It also recollects information of each operation, such as the spread of each swap and the total_comission_amount.

The issue is that the value total_comission_amount is invalid. This is because the code is adding amounts of different tokens; for example, adding the comission_amount of swapping token1 for token2 to the comission_amount of swapping token2 for token3, which are quantities in different tokens.

The same issue happens in the simulate_reverse_swap function.

Impact: The value returned by simulate_swap_response.total_commission_amount is an invalid amount since amounts of different tokens cannot be added together.

Recommendation: Similar to spread, the values of commission_amount should be returned in a vector of values.