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

Vulnerability details

Impact

If Canto price reaches a large amount, small swaps will always revert.

Proof of Concept

This is the current user flow to onboard users into the Canto network:

User Flow

User transfers assets to the Canto network through Gravity Bridge
Check recipient address's Canto balance
If the balance is less than the minimum threshold (e.g. 4canto), swap the assets to Canto using the coinswap module
If the remaining assets are registered as a ERC20, convert them to ERC20.

        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 {

Right now, Canto is worth $0.11 USDC, which is pretty affordable to swap. However, if it reach an exorbitant amount of price (like ETH price for example), then users will not have enough funds to reach the threshold of 4 Canto in their wallets, which may force all swaps to swap to Canto first. If the swap amount is lesser than 4 Canto worth, then the swap will fail, and it may affect many people trying to swap small amounts into the Canto network.

Tools Used

Manual Review

Recommended Mitigation Steps

Recommending setting AutoSwapThreshold to a dynamic integer, in case 4 Canto gets too expensive.

Assessed type

Other

tkkwon1998 commented 1 year ago

AutoSwapThreshold is a parameter that can be modified through governance. If the price of canto changes drastically, this param will be adjusted accordingly. Although having a dynamically adjusting swap threshold would be a good solution, it would likely require some kind of oracle which increases complexities and failure points.

c4-sponsor commented 1 year ago

tkkwon1998 marked the issue as sponsor disputed

c4-judge commented 1 year ago

0xean changed the severity to QA (Quality Assurance)