code-423n4 / 2024-02-hydradx-findings

1 stars 0 forks source link

[M05] `MinimumPoolLiquidity` check affects tokens with different decimals differently #89

Open c4-bot-1 opened 8 months ago

c4-bot-1 commented 8 months ago

Lines of code

https://github.com/code-423n4/2024-02-hydradx/blob/603187123a20e0cb8a7ea85c6a6d718429caad8d/HydraDX-node/pallets/omnipool/src/lib.rs#L471-L474 https://github.com/code-423n4/2024-02-hydradx/blob/603187123a20e0cb8a7ea85c6a6d718429caad8d/HydraDX-node/pallets/omnipool/src/lib.rs#L580-L583

Vulnerability details

Impact

The MinimumPoolLiquidity check is used to ensure there is a minimum amount of liquidity for each token in the omnipool. The issue is that this same limit is used for all tokens, regardless of their token decimals. Thus if the MinimumPoolLiquidity is set to 1e6, while that is an acceptable amount for tokens with 12 decimals, that is a very large number of tokens for a token with only 2 decimals.

This can prevent liquidity providers from providing small amounts of liquidity with low decimal tokens.

Proof of Concept

In the code, the MinimumPoolLiquidity is checked against the amount of tokens. Meanwhile in the stableswap contracts, this same value is checked against the amount of minted shares, which can be expected to be of standard decimals, since it is controlled by the protocol itself.

Tools Used

Manual Review

Recommended Mitigation Steps

Scale up MinimumPoolLiquidity according to the token of the decimals, or put the check on the amount of shares minted instead of the assets put in like done in stableswap.

Assessed type

ERC20

c4-pre-sort commented 8 months ago

0xRobocop marked the issue as primary issue

c4-pre-sort commented 8 months ago

0xRobocop marked the issue as sufficient quality report

c4-sponsor commented 8 months ago

enthusiastmartin (sponsor) disputed

enthusiastmartin commented 8 months ago

It is by design. It is soft limit .

OpenCoreCH commented 8 months ago

Valid improvement for suggestion, security impact seems very limited and is not demonstrated

c4-judge commented 8 months ago

OpenCoreCH changed the severity to QA (Quality Assurance)

c4-judge commented 8 months ago

OpenCoreCH marked the issue as grade-b