Malicious First depositor can DOSed the add liquidity function in coinswap module

[howlbot-integration]

Lines of code

https://github.com/code-423n4/2024-05-canto/blob/main/canto-main/x/coinswap/keeper/keeper.go#L119-L250

Vulnerability details

Detailed description

The add liquidity function in keeper.go under coinswap module implements the deposit of tokens in exchange of liquidity shares. However, the function is vulnerable to donation attack wherein the succeeding depositors won't be successfully add liquidity because the standard reserve pool has been manipulated.

Let's look at line 173 below, this can be manipulated and cause error of MaxStandardCoinPerPool in line 193. This can be problematic because it exceeded the limitation but in reality, there is no equivalent of actual proper deposit of liquidity. It is just simply manipulated by direct transfer or deposit from malicious actor to the reserve pool.

File: keeper.go
173: 		standardReserveAmt := balances.AmountOf(standardDenom) // @audit this can be manipulated by directly sending standard tokens to the pool reserves
174: 		tokenReserveAmt := balances.AmountOf(msg.MaxToken.Denom) // pertains to deposited token like btc or eth or usdc balance of the pool
175: 		liquidity := k.bk.GetSupply(ctx, pool.LptDenom).Amount// pertains to total supply of LP token of the pool
176: 
~skipping other code lines
192: 		} else {// if pool exists with deposited amount
193: 			if standardReserveAmt.GTE(params.MaxStandardCoinPerPool) { // If standard reserve is more than or equal to max standard coin it will become error
194: 				return sdk.Coin{}, errorsmod.Wrap(types.ErrMaxedStandardDenom, fmt.Sprintf("pool standard coin is maxed out: %s", params.MaxStandardCoinPerPool.String()))
195: 			}

Impact

Future deposits in a manipulated pool won't be possible. This will disrupt the deposit operation of the pool in which the users can't deposit no more because of manipulated restriction in each pool made by the attacker.
This can be problematic because the attacker can do this repeatedly in other pools and disrupt the whole protocol.

Proof of Concept

Consider this scenario:

1. User A, a well funded malicious first depositor add liquidity by 1 wei token to the pool in exchange of 1 wei share
2. User B, a next depositor add liquidity to the same pool with good intent amounting to 100 tokens.
3. User A see the transaction and frontrun it by directly sending large amount of standard tokens to the reserve pool equal to the MaxStandardCoinPerPool. In other words, the standard pool reserves limit is already met.
4. User B transaction will fail because of restriction of MaxStandardCoinPerPool.
5. Other next users also attempted to deposit but no avail due to same restriction on step 4.

Please be reminded that this can be done in other pools and the impact can be devastating and eventually the protocol lost users.

Tools Used

Manual Review

Recommended Mitigation Steps

Implement a mechanism in which the first depositor will be prevented to manipulate the reserves.

Assessed type

Other

[c4-judge]

3docSec changed the severity to QA (Quality Assurance)

[c4-judge]

3docSec marked the issue as grade-b