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

1 stars 0 forks source link

Incorrect setting of EthIBCDenom invalidates risk management limits #48

Closed code423n4 closed 1 year ago

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 documentation, it is stated that:

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.

However, in the code it defined as:

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

This is incorrect since ETH has 18 decimals so the NewCoin with 17 decimals will be 0.1 ETH and not 0.01 ETH.

Since the maximum actual ETH input is 10x the stated limit and, at current prices, 18x the USDC/USDT limit, it is clear that the risk management intended by the protocol through input limits, is ineffective.

Proof of Concept

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

https://github.com/code-423n4/2023-06-canto/blob/a4ff2fd2e67e77e36528fad99f9d88149a5e8532/README.md

Tools Used

Manuel Review

Recommended Mitigation Steps

Change the NewCoin Denom to 16 decimals.

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

Assessed type

Decimal

c4-pre-sort commented 1 year ago

JeffCX marked the issue as duplicate of #8

c4-judge commented 1 year ago

0xean marked the issue as satisfactory

c4-judge commented 1 year ago

0xean changed the severity to 3 (High Risk)