search

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

1 stars 0 forks source link

The amount of Canto to swap in onboarding should be adjusted according to the user's Canto balance #87

Open code423n4 opened 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#L88-L93

Vulnerability details

Impact

The current implementation may result in onboarding auto-swaps being performed too often, causing many users to make unnecessary Canto swaps.

Proof of Concept

The purpose of onboarding is to provide the user's account with canto funds to cover the gas fee for the first 1/2 transaction, and 4 canto (DefaultAutoSwapThreshold) is currently considered a reasonable minimum.

The logic of the current code implementation in onboardingKeeper.OnRecvPacket() is as follows:

    swapCoins := sdk.NewCoin(standardDenom, autoSwapThreshold)
    standardCoinBalance := k.bankKeeper.SpendableCoins(ctx, recipient).AmountOf(standardDenom)
    swappedAmount := sdk.ZeroInt()

    if standardCoinBalance.LT(autoSwapThreshold) {
        swappedAmount, err = k.coinswapKeeper.TradeInputForExactOutput(......)

So if there are 3.9 Canto in the recipient address, the onboarding Middleware will swap for another 4 Canto for the recipient too. Finnaly, the recipient will have 7.9 Canto, which is much more than 4 Canto. This is probably an unnecessary and redundant exchange for the user.

Tools Used

VS Code

Recommended Mitigation Steps

We should change the swap amount in onboardingKeeper.OnRecvPacket(), adjust it according to the recipient's Canto balance.

We can consider set

  1. set the output amount to max(1e17, autoSwapThreshold - balanceOf(receiver))
  2. set the output amount to max(1e17, autoSwapThreshold + bufferAmount - balanceOf(receiver))
    • bufferAmount < 4e18 is used to swap a bit more each time, which may reduces subsequent swaps in future bridges.

Assessed type

Other

c4-pre-sort commented 1 year ago

JeffCX marked the issue as low quality report

c4-pre-sort commented 1 year ago

JeffCX marked the issue as high quality report

c4-pre-sort commented 1 year ago

JeffCX marked the issue as primary issue

tkkwon1998 commented 1 year ago

Acknowledged, this is a known limitation on the current iteration of the module.

c4-sponsor commented 1 year ago

tkkwon1998 marked the issue as sponsor acknowledged

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-b