code-423n4 / 2023-06-canto-findings

1 stars 0 forks source link

The Swap action will always fail if the value of the deposited IBC asset is less than the value of autoSwapThreshold Canto #37

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-06-canto/blob/a4ff2fd2e67e77e36528fad99f9d88149a5e8532/Canto/x/onboarding/keeper/ibc_callbacks.go#L92-L108

Vulnerability details

Impact

The Swap action will always fail if the value of the deposited IBC asset < the value of autoSwapThreshold Canto. This is not a bug, but it's very inconvenient for users and makes the goal of the onboarding module fail.

Let's asssume that: 1 Canto = 0.1 USDC, autoSwapThreshold = 4 Canto (value at ~0.4 USDC), Canto balance of this User = 0 User flow:

  1. User bridges 0.3 USDC to Canto using IBC transfer through Gravity Bridge.
  2. Check whether the User's Canto balance < autoSwapThreshold or not. Because 0 < 4 true, the next step is to perform TradeInputForExactOutput() with autoSwapThreshold as the exact output params, but this function will fail because can't swap 0.3 USDC to 4 Canto.
  3. All amounts of USDC will be converted from an IBC asset to an ERC20 asset. This conversation will be successful.
  4. This user bridges n times 0.3 USDC but he will never have any Canto to pay the initial gas (the goal of the onboarding module will fail)

Proof of Concept

    if standardCoinBalance.LT(autoSwapThreshold) {
        swappedAmount, err = k.coinswapKeeper.TradeInputForExactOutput(ctx, coinswaptypes.Input{Coin: transferredCoin, Address: recipient.String()}, coinswaptypes.Output{Coin: swapCoins, Address: recipient.String()})
        if err != nil {
            logger.Error("failed to swap coins", "error", err)
        } else {
            ctx.EventManager().EmitEvent(
                sdk.NewEvent(
                    coinswaptypes.EventTypeSwap,
                    sdk.NewAttribute(coinswaptypes.AttributeValueAmount, swappedAmount.String()),
                    sdk.NewAttribute(coinswaptypes.AttributeValueSender, recipient.String()),
                    sdk.NewAttribute(coinswaptypes.AttributeValueRecipient, recipient.String()),
                    sdk.NewAttribute(coinswaptypes.AttributeValueIsBuyOrder, strconv.FormatBool(true)),
                    sdk.NewAttribute(coinswaptypes.AttributeValueTokenPair, coinswaptypes.GetTokenPairByDenom(transferredCoin.Denom, swapCoins.Denom)),
                ),
            )
        }
    }

Permalink

Tools Used

VSCode

Recommended Mitigation Steps

The team might consider using TradeExactInputForOutput() with transferredCoin as the exact input params in case the value of transferredCoin IBC assets < the value of autoSwapThreshold Canto to mitigate this inconvenience for the User. Still using TradeInputForExactOutput() with autoSwapThreshold as the exact output params in the remaining case.

Assessed type

Other

c4-pre-sort commented 1 year ago

JeffCX marked the issue as low quality report

JeffCX commented 1 year ago

this function will fail because can't swap 0.3 USDC to 4 Canto.

given the CANTO price is 0.1 USD,

the report describes a expected behavior

c4-judge commented 1 year ago

0xean marked the issue as unsatisfactory: Invalid