code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-06-canto/blob/a4ff2fd2e67e77e36528fad99f9d88149a5e8532/Canto/x/coinswap/types/params.go#L34

Vulnerability details

Impact

In the "coinswap" module a limit is in place for avoiding large swaps and their potential to manipulate the price in a low-liquidity scenario.

The spec says:

For risk management purposes, a swap will fail if the input coin amount exceeds a pre-defined limit (10 USDC, 10 USDT, 0.01 ETH) or if the swap amount limit is not defined.

The limit as stated in the spec is of $10~15 which is about 1% of the maximum liquidity allowed in CANTO (10,000 canto ~= $1,000). However, the ETH/Canto pool has a 0.1ETH ~= $150 swap limit that is 15x higher. This discrepancy, with the "0.01" mention in the spec, gives high confidence that the value in the code is not the intended one, and is an off-by-one error.

Proof of Concept

The following failing test case would pass if the amounts respected the spec:

package types

import (
    "github.com/stretchr/testify/assert"
    "testing"
)

func TestDefaultParams(t *testing.T) {
    // 10 USDC (10 + 6 zeros)
    assert.Equal(t, 8, len(DefaultMaxSwapAmount.AmountOfNoDenomValidation(UsdcIBCDenom).String()))

    // 10 USDT (10 + 6 zeros)
    assert.Equal(t, 8, len(DefaultMaxSwapAmount.AmountOfNoDenomValidation(UsdcIBCDenom).String()))

    // 0.01 ETH (1 + (18-2=) 16 zeros)
    assert.Equal(t, 17, len(DefaultMaxSwapAmount.AmountOfNoDenomValidation(EthIBCDenom).String()))
}

Tools Used

Visual inspection

Recommended Mitigation Steps

Change params.go#L34 line to

        sdk.NewCoin(EthIBCDenom, sdk.NewIntWithDecimal(1, 16)),

Assessed type

Decimal

c4-pre-sort commented 1 year ago

JeffCX marked the issue as primary issue

JeffCX commented 1 year ago

The report highlights a misconfiguration, the setting for ETH limit is 0.01 ETH while the current setting is 0.1 ETH, I believe the report worth sponsor's review

tkkwon1998 commented 1 year ago

Agreed, this issue is valid as limit is 10x higher than it should be. Although losses are still minimal (0.1 eth at most), I agree with high risk since funds can be lost if pools are manipulated.

c4-sponsor commented 1 year ago

tkkwon1998 marked the issue as sponsor confirmed

c4-judge commented 1 year ago

0xean marked the issue as satisfactory