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-035] Lack of validation on total_shares #229

Closed gangov closed 7 months ago

gangov commented 7 months ago

files: contracts/pool/src/contract.rs locations: withdraw_liquidity

In withdraw_liquidity, the number of total_shares is checked to ensure it is non-zero before computing share_ratio to avoid a divide-by-zero error:

let mut share_ratio = Decimal::zero();
let total_shares = utils::get_total_shares(&env); 
if total_shares != 0i128 {
  share_ratio = Decimal::from_ratio(share_amount, total_shares); 
}

total_shares check and share_ratio computation.

However, if total_shares is zero, then there is no way that any liquidity can be withdrawn (as there isn’t any liquidity in the pool). So, if total_shares is zero here and the transfer of share tokens has occurred, then there is an error in the protocol.

Impact: Performing the zero check on total_shares without issuing an error if total_shares is zero misses an opportunity to report a serious protocol error if it has occurred.

Recommendation: Ensure that total_shares is not zero and issue a panic otherwise.