Submitted by J4X, also found by carrotsmuggler and 3docSec
The StableSwap AMM of the HydraDx protocol implements safeguards against low liquidity so that too high price fluctuations are prevented, and manipulating the price becomes harder. These safeguards are enforced based on the MinPoolLiquidity which is a constant that describes the minimum liquidity that should be in a pool. Additionally, a pool is allowed to have a liquidity of 0, which would occur in the case of the creation of the pool, or by users withdrawing all their liquidity. This could also be defined as an invariant.
totalPoolIssuance(poolId) >= MinPoolLiquidity || totalPoolIssuance(poolId) == 0.
When a user wants to withdraw his liquidity, he can use either the remove_liquidity_one_asset() function or the withdraw_asset_amount() function.
remove_liquidity_one_asset():
To ensure holding the invariant 2 checks are implemented in the remove_liquidity_one_asset() function. The first checks if the user either leaves more than MinPoolLiquidity shares in the pool or withdraws all his shares:
The second checks if the total liquidity in the pool would fall below the intended amount of shares:
These two checks work perfectly at holding the invariant at all times.
withdraw_asset_amount():
Unfortunately, the second function for withdrawing liquidity withdraw_asset_amount() omits one of the checks. The function only checks if the user either withdraws all his shares or leaves more than the MinPoolLiquidity shares.
One might state now that this could never break the invariant, as if every users shares are either more than MinPoolLiquidity or zero, the total liquidity can never fall below MinPoolLiquidity without being 0. Unfortunately, this approach forgets that users can transfer their shares to other addresses. This allows a user to transfer an amount as low as 1 share to another address, and then withdraw all his shares. As the check would only ensure that he is withdrawing all his shares it would pass. If he was the only liquidity provider, there now would only be 1 share of liquidity left in the pool breaking the invariant of: totalPoolIssuance(poolId) >= MinPoolLiquidity.
The issue allows a user to break the invariant about the MinPoolLiquidity and either push the pool into a state where it can easily be manipulated, or prevent other users from withdrawing their shares.
There are 2 ways how this could be exploited:
1. Breaking the invariant and letting the pool Liquidity fall below MinPoolLiquidity
A malicious LP could abuse this functionality to push the pool into a state where its total liquidity is below MinPoolLiquidity, and could be as low as 1 share, allowing for easy price manipulation. To achieve this the attacker would do the following:
2. DOSing withdrawing through remove_liquidity_one_asset for others
Lets consider a case where there are only 2 liquidity providers and one of them is malicious and wants to prevent the other from withdrawing through remove_liquidity_one_asset(). This could for example be the case if the other is a smart contract, that can only withdraw through this function.
The issue can be mitigated by also adding a check for the total pool liquidity to withdraw_asset_amount():
enthusiastmartin (HydraDX) confirmed, but disagreed with severity and commented:
Lambda (judge) commented:
castle_chain (warden) commented:
J4X (warden) commented:
