Token pairs that are not whitelisted can be created as a pool

[code423n4]

Lines of code

https://github.com/code-423n4/2023-06-canto/blob/main/Canto/x/coinswap/keeper/pool.go#L15

Vulnerability details

Class

Medium

Impact

In the docs:

Only token pairs on the whitelist can be created as a pool. Pool creation fails if the token pair is not on the whitelist.

However, there is no logic that prevents from creating non-whitelisted pairs. The check is only happening in In OnRecvPacket when converting the coins.

    pair, _ := k.erc20Keeper.GetTokenPair(ctx, pairID)
    if !pair.Enabled {
        // no-op: continue with the rest of the stack without conversion
        return ack
    }

Proof of Concept

• It doesn't check if the pair is allowed

// CreatePool create a liquidity that saves relevant information about popular pool tokens
func (k Keeper) CreatePool(ctx sdk.Context, counterpartyDenom string) types.Pool {
    sequence := k.getSequence(ctx)
    lptDenom := types.GetLptDenom(sequence) // lpt-1 where 1 is sequence
    pool := &types.Pool{
        Id:                types.GetPoolId(counterpartyDenom), // pool-usdc where usdc is counterpartyDenom
        StandardDenom:     k.GetStandardDenom(ctx),
        CounterpartyDenom: counterpartyDenom,
        EscrowAddress:     types.GetReservePoolAddr(lptDenom).String(),
        LptDenom:          lptDenom,
    }
    k.setSequence(ctx, sequence+1)
    k.setPool(ctx, pool)
    return *pool
}

https://github.com/code-423n4/2023-06-canto/blob/main/Canto/x/coinswap/keeper/pool.go#L15

• CreatePool is called on liqudityCreate

  // calculate amount of UNI to be minted for sender
  // and coin amount to be deposited
  if !exists {
      // deduct the user's fee for creating a Liquidity pool
      if err := k.DeductPoolCreationFee(ctx, sender); err != nil {
          return sdk.Coin{}, err
      }
  
      mintLiquidityAmt = msg.ExactStandardAmt
  
      if mintLiquidityAmt.GT(params.MaxStandardCoinPerPool) {
          return sdk.Coin{}, sdkerrors.Wrap(types.ErrMaxedStandardDenom, fmt.Sprintf("liquidity amount not met, max standard coin amount: no bigger than %s, actual: %s", params.MaxStandardCoinPerPool.String(), mintLiquidityAmt.String()))
      }
  
      if mintLiquidityAmt.LT(msg.MinLiquidity) {
          return sdk.Coin{}, sdkerrors.Wrap(types.ErrConstraintNotMet, fmt.Sprintf("liquidity amount not met, user expected: no less than %s, actual: %s", msg.MinLiquidity.String(), mintLiquidityAmt.String()))
      }
  
      depositToken = sdk.NewCoin(msg.MaxToken.Denom, msg.MaxToken.Amount)
      pool = k.CreatePool(ctx, msg.MaxToken.Denom)

https://github.com/code-423n4/2023-06-canto/blob/main/Canto/x/coinswap/keeper/keeper.go#L149

Tools Used

Manual analysis

Recommended Mitigation Steps

Check if the pair is whitelisted before creating the pool.

Assessed type

Other

[c4-pre-sort]

JeffCX marked the issue as low quality report

[c4-pre-sort]

JeffCX marked the issue as primary issue

[0xean]

if !params.MaxSwapAmount.AmountOf(msg.MaxToken.Denom).IsPositive() {
        return sdk.Coin{}, sdkerrors.Wrapf(types.ErrInvalidDenom,
            "MaxToken %s is not registered in max swap amount", msg.MaxToken.Denom)
    }

I believe the expected check occurs here, will leave open for sponsor comment before closing

[0xean]

@c4-sponsor

[tkkwon1998]

if !params.MaxSwapAmount.AmountOf(msg.MaxToken.Denom).IsPositive() {
      return sdk.Coin{}, sdkerrors.Wrapf(types.ErrInvalidDenom,
          "MaxToken %s is not registered in max swap amount", msg.MaxToken.Denom)
  }

I believe the expected check occurs here, will leave open for sponsor comment before closing

yes confirmed, whitelist check happens in addLiquidity function

[c4-judge]

0xean marked the issue as unsatisfactory: Invalid