A malicious pool creator or first depositor can make depositing and removing liquidity unfavorable for other users.

[howlbot-integration]

Lines of code

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

Vulnerability details

Impact

Users who add liquidity to the pool can lose funds and also, amount of max tokens that users can deposit would be severely truncated for a period of time.

Proof of Concept

There are 2 ways for the pool creator or first depositor to make using a pool unfavourable to users.

1. On creation of the pool, user mints 100,000 tokens and deposits just 1 deposit token. This causes the pool to have a liquidity balance of 100,000, standard reserve balance of 100,000 but a tokenReserveAmount of 1. Subsequently when users want to deposit tokens their token deposit would be severely limited.

else {
			if standardReserveAmt.GTE(params.MaxStandardCoinPerPool) {
				return sdk.Coin{}, errorsmod.Wrap(types.ErrMaxedStandardDenom, fmt.Sprintf("pool standard coin is maxed out: %s", params.MaxStandardCoinPerPool.String()))
			}

			maxStandardInputAmt := sdkmath.MinInt(msg.ExactStandardAmt, params.MaxStandardCoinPerPool.Sub(standardReserveAmt))
			mintLiquidityAmt = (liquidity.Mul(maxStandardInputAmt)).Quo(standardReserveAmt)
			if mintLiquidityAmt.LT(msg.MinLiquidity) {
				return sdk.Coin{}, errorsmod.Wrap(types.ErrConstraintNotMet, fmt.Sprintf("liquidity amount not met, user expected: no less than %s, actual: %s", msg.MinLiquidity.String(), mintLiquidityAmt.String()))
			}

			@> depositAmt := (tokenReserveAmt.Mul(maxStandardInputAmt)).Quo(standardReserveAmt).AddRaw(1)
			depositToken = sdk.NewCoin(msg.MaxToken.Denom, depositAmt)
			standardCoin = sdk.NewCoin(standardDenom, maxStandardInputAmt)
			if depositAmt.GT(msg.MaxToken.Amount) {
				return sdk.Coin{}, errorsmod.Wrap(types.ErrConstraintNotMet, fmt.Sprintf("token amount not met, user expected: no more than %s, actual: %s", msg.MaxToken.String(), depositToken.String()))
			}

Let's assume a user wants to make a standard deposit of 50,000 tokens. The deposit token is calculated as 1 * 50,000/100,000+1. This gives the user the ability to only deposit 1 max tokens despite probably expecting to deposit more.
This limits the addliquidity functionality of that pool for many transactions until tokenReserveAmount eventually becomes a significant value. This action can serve to deter users from using the pool.
2. Pool creator creates a pool with standardInputAmount of 100,000 and tokenReserve of 100,000. Then sends 50,000 standard coins directly to the pool. This causes the standardReserveAmount to become larger than the lpLiquidity. When another user deposits 100,000 tokens, they get 66666 of mintliquidity and deposit 66667 of max tokens. If that user wants to remove their liquidity, they only get 99,999 of their standardCoin deposit back, having lost 1 token to the pool. The original pool creator retains all his assets. If sufficient number of users lose assets to the pool, the pool creator gains the lost assets added to his liquidity.

Tools Used

Manual Review

Recommended Mitigation Steps

1. For initial Pool creator there should be validation that the max token deposit and standardReserveAmount do not surpass a given threshold.
   For the 2nd, consider preventing direct transfers of tokens to the pool. This would ensure correct accounting

Assessed type

Math

[c4-judge]

3docSec changed the severity to QA (Quality Assurance)

[c4-judge]

3docSec marked the issue as grade-b