Submitted by castle_chain
https://github.com/code-423n4/2024-02-hydradx/blob/603187123a20e0cb8a7ea85c6a6d718429caad8d/HydraDX-node/pallets/omnipool/src/lib.rs#L612-L621
This vulnerability will lead to prevent any liquidity provider from adding liquidity and prevent them from minting new shares; so this is considered a huge loss of funds for the users and the protocol.
Adding a new token to the omnipool requires an initial liquidity deposit. This initial deposit mints the first batch of shares. Subsequent liquidity additions mint new shares proportionally to the existing total shares, ensuring a fair distribution based on the pools current size.
In the function remove_liquidity it is allowed to remove all amount of liquidity from the pool, which means burning all amount of the shares from the pool, as shown here.
The function calculate_remove_liquidity_state_changes calculates the shares to be burnt and the delta_update function removes them.
If all liquidity shares have been removed from any pool, protocol shares and user shares are removed, making the asset_reserve equal to zero and shares equal to zero, which will prevent any liquidity from been added because the function add_liquidity does not handle the situation where there is no liquidity in the pool. As shown in the add_liquidity function, it calls calculate_add_liquidity_state_changes which calculates the shares to be minted to the LP as shown here.
The state of the pool after all liquidity have been removed asset_reserve = 0, shares = 0. Since there is no liquidity in the pool so the reserve_hp will be equal to zero, so this part will always return error, so this function will always revert.
Even if any user donates some assets to prevent this function from reverting, the liquidity provider will always receive zero shares, since the delta_shares_hp will always equal to zero, which is considered loss of funds for the user and the protocol.
The bad state that will cause this vulnerability:
Token has been added to the pool but all liquidity has been removed from it.
Consider adding this test in remove_liquidity.rs test file, and then run it to see the logs:
The logs will be:
This illustrates that the add_liquidity function failed after lp2 and lp1 removed their entire liquidity from the pool.
VS Code
Add a special behaviour to the function add_liquidity to handle the situation of no initial liquidity. The mitigation can be done by:
As happening in the function add_token() here.
Context
Lambda (judge) decreased severity to Medium and commented:
castle_chain (warden) commented:
castle_chain (warden) commented:
Lambda (judge) commented:
enthusiastmartin (HydraDX) disputed and commented:
Note: For full discussion, see here.
