c4-bot-7 commented 6 months ago

Lines of code

https://github.com/code-423n4/2024-02-hydradx/blob/main/HydraDX-node/pallets/omnipool/src/lib.rs#L1548

Vulnerability details

Impact

remove_token can't be called on all the assets (function unavailable).

Proof of Concept

remove_token is a function which is called by AuthorityOrigin to remove a token asset. In this function, we will make sure there are no shares except protocol ones, otherwise SharesRemaining error is returned. And, there is no way to force the withdrawal of a user's shares.

So, a malicious user can add dust liquidity (MinimumPoolLiquidity) whenever a new token asset is added to the omnipool, making the remove_token totally unavailable, which may affect the tokenomics when the community decides to remove an asset.

Tools Used

Manual Review.

Recommended Mitigation Steps

In the remove_liquidity, the authority can withdraw an account's shares (asset_state.tradable == Tradability::FROZEN) and send the corresponding assets to it.

Assessed type

Context

c4-pre-sort commented 6 months ago

0xRobocop marked the issue as insufficient quality report

c4-pre-sort commented 6 months ago

0xRobocop marked the issue as primary issue

OpenCoreCH commented 6 months ago

It is generally a good idea that the removal is not possible in this case because this would be a large centralization risk, otherwise. The duplicates of the issue present some alternative designs that would be possible, although it is not clear if they would be better. Downgrading to QA because of design aspect.

c4-judge commented 6 months ago

OpenCoreCH changed the severity to QA (Quality Assurance)