search

code-423n4 / 2022-03-biconomy-findings

0 stars 0 forks source link

Liquidity providers unable to remove liquidity when the pool is in deficit state #93

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-03-biconomy/blob/04751283f85c9fc94fb644ff2b489ec339cd9ffc/contracts/hyphen/LiquidityProviders.sol#L388 https://github.com/code-423n4/2022-03-biconomy/blob/04751283f85c9fc94fb644ff2b489ec339cd9ffc/contracts/hyphen/LiquidityProviders.sol#L392

Vulnerability details

Impact

LP token holders can not redeem their tokens when the pool is in the deficit state, i.e. currentLiquidity << providedLiquidity. This is due to that LP shares are computed based on providedLiquidity and the actual available pool balance is based on currentLiquidity.

Proof of Concept

When a high valued withdrawal happens in the liquidity pool of the destination chain, the current liquidity will be reduced when the executor calls sendFundsToUser https://github.com/code-423n4/2022-03-biconomy/blob/04751283f85c9fc94fb644ff2b489ec339cd9ffc/contracts/hyphen/LiquidityPool.sol#L285

and the pool contract balance will also be reduced by the same amount. The pool reached a deficit state with provided liquidity much bigger than current liquidity.

The LP shares are computed based on the value of totalReserve that is roughly equivalent to totalLiquidity + LpFees. In a deficit state, totalReserve could be much bigger than the available pool balance (up to 90% since max fee is 10%). If the LP token holder wants to redeem his shares,

        _decreaseCurrentLiquidity(_tokenAddress, _amount);

will underflow and revert and 

        _transferFromLiquidityPool(_tokenAddress, _msgSender(), amountToWithdraw);

will revert because there is not enough balance.

Recommended Mitigation Steps

This is a tricky problem. On one hand, separating currentLiquidity from providedLiquidity made sure that by bridging tokens over, it will not inflate or deflate the pool. On the other hand, decoupling the two made it hard to compute the actual available liquidity to redeem LP shares. One may need to think through this a bit more.

ankurdubey521 commented 2 years ago

Liquidity Providers will be able to withdraw their funds as long as they're sufficient currentLiquidity in the pool, as you mentioned. This will be the case when all pools are balanced, ie the current liquidity is very close to the supplied liquidity. By design, hyphen liquidity pools incentivize people to rebalance the pools by providing rewards from the incentive pool, so we believe this should not be that big of an issue in practice.

pauliax commented 2 years ago

A valid concern, and even though per the sponsor's comment it should not be a problem in practice, a hypothetical path of risk still exists so I would like to leave this as of Medium severity issue.