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

1 stars 0 forks source link

SWAP AMOUNT OF CANTO IS HARDCODED TO `4 Canto` IRRESPECITVE OF THE `Canto` BALANCE OF THE `Recipient`, WHICH COULD BE FURTHER DISADVANTAGEOUS TO THE `Recipient` IF THE CANTO PRICE INCREASES IN THE FUTURE #52

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-06-canto/blob/main/Canto/x/onboarding/keeper/ibc_callbacks.go#L87-L88 https://github.com/code-423n4/2023-06-canto/blob/main/Canto/x/onboarding/keeper/ibc_callbacks.go#L92-L93

Vulnerability details

Impact

In the ibc_callbacks.OnRecvPacket function, the IBC transfer asset is swapped to canto token if the canto balance of the recipient is less than the AutoSwapThreshold. But the issue is that the amount of swapped IBC asset is always equal to the AutoSwapThreshold Canto amount. Hence there is a disadvantage to the Recipient since he is receiving a less amount of ERC20 tokens from the IBC asset tokens conversion as per the following scenario:

Consider the following scenario: Recipient A:

  1. Has a Canto balance of 3 in his account.
  2. Assume that current AutoSwapThreshold is the default value of 4.
  3. standardCoinBalance.LT(autoSwapThreshold) check is performed in the ibc_callbacks.OnRecvPacket function.
  4. Since the above condition is true k.coinswapKeeper.TradeInputForExactOutput is called and input amount of transferred tokens is swapped which would output the exact amount of AutoSwapThreshold (4) amount of Canto tokens.
  5. But the difference between the AutoSwapThreshold and the canto balance was 4 - 3 = 1.
  6. Hence even if an amount equaling 1 Canto of input tokens was swapped it can still fulfill the AutoSwapThreshold requirement of 4, since the original balance was 3.
  7. But the amount equaling 4 Canto of input tokens was swapped, increasing the canto balance to 7.
  8. Hence the recipient A will receive less amount of ERC20 tokens from conversions since extra amount of input tokens was initially swapped for Canto Token.

Recipient B:

  1. Has a Canto balance of 4 in his account.
  2. Assume that current AutoSwapThreshold is the default value of 4.
  3. standardCoinBalance.LT(autoSwapThreshold) check is performed in the ibc_callbacks.OnRecvPacket function.
  4. Since the above conditions is false there won't be any swap happening between transferred token and Canto token.
  5. The recipient will get the entire amount of IBC transferred token amount converted to ERC20 tokens.

As it is shown above Recipient B receives more ERC20 tokens after conversion than Recipient A, due to the swap and conversion logic of the ibc_callbacks.OnRecvPacket function. The Recipient B can have relatively more ERC20 tokens converted from the same amount of IBC input transferred token just because he has 1 extra canto token in his account balance than the Recipient A.

Furthermore Recipient A will have less ERC20 tokens to be used in the dapps of the canto network, since more IBC tokens were swapped for Canto initially to fulfill the AutoSwapThreshold requirement.

This could be more problematic if the Canto token increases in price in the future. As per current pricing 1 Canto = 0.1135 USD. But in the future if 1 Canto = 10USDC or more, the impact borne by the respective Recipient will be high due to the hardcoded AutoSwapThreshold (4 Canto) threshold requirement.

Proof of Concept

autoSwapThreshold := k.GetParams(ctx).AutoSwapThreshold
swapCoins := sdk.NewCoin(standardDenom, autoSwapThreshold)

https://github.com/code-423n4/2023-06-canto/blob/main/Canto/x/onboarding/keeper/ibc_callbacks.go#L87-L88

if standardCoinBalance.LT(autoSwapThreshold) {
    swappedAmount, err = k.coinswapKeeper.TradeInputForExactOutput(ctx, coinswaptypes.Input{Coin: transferredCoin, Address: recipient.String()}, coinswaptypes.Output{Coin: swapCoins, Address: recipient.String()})

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

Tools Used

VSCode and Manual Review

Recommended Mitigation Steps

If the AutoSwapThreshold > canto balance of recipient, It is recommended to calculate the difference between the AutoSwapThreshold and canto balance of recipient and only swap an amount from the input transferred tokens that would output the exact amount of the difference of the above two parameters.

For example in the scenario associate with Recipient A, it is recommended to swap amount equaling to 1 Canto (since the difference is 1) rather than swapping an amount equal to 4 Canto.

Assessed type

Other

c4-pre-sort commented 1 year ago

JeffCX marked the issue as duplicate of #87

c4-judge commented 1 year ago

0xean changed the severity to QA (Quality Assurance)

c4-judge commented 1 year ago

0xean marked the issue as grade-c