Canto - 3docSec's results

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

• the swap limit MaxSwapAmount is easily circumvented: one could chain multiple types.MsgSwapOrder in the same transaction and achieve atomically the same result as swapping a larger amount with a single message

• x/onboarding/keeper/ibc_callbacks.go function OnRecvPacket:

  • the onboarding function moves on with ERC20 conversion even if there are no coins left (transferredCoin - swappedAmount = 0). This can lead to sending 0 coins to the erc20 module for swapping, which is inconsistent with the erc20 module expectations (the MsgConvertCoin.validateBasic() would fail on zero amounts)