code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-06-canto/blob/a4ff2fd2e67e77e36528fad99f9d88149a5e8532/Canto/x/coinswap/keeper/swap.go#L210-L218 https://github.com/code-423n4/2023-06-canto/blob/a4ff2fd2e67e77e36528fad99f9d88149a5e8532/Canto/x/coinswap/types/params.go#L12-L16 https://github.com/code-423n4/2023-06-canto/blob/a4ff2fd2e67e77e36528fad99f9d88149a5e8532/Canto/x/coinswap/types/params.go#L31-L36

Vulnerability details

Impact

Hard coding UsdcIBCDenom, UsdtIBCDenom and EthIBCDenom may lead to panic in the future

Proof of Concept

// Only those IBC denom tokens are allowed to convert to Canto.
const (
    UsdcIBCDenom = "ibc/17CD484EE7D9723B847D95015FA3EBD1572FD13BC84FB838F55B18A57450F25B"
    UsdtIBCDenom = "ibc/4F6A2DEFEA52CD8D90966ADCB2BD0593D3993AB0DF7F6AEB3EFD6167D79237B0"
    EthIBCDenom  = "ibc/DC186CA7A8C009B43774EBDC825C935CABA9743504CE6037507E6E5CCE12858A"
)

According References UsdcIBCDenom, UsdtIBCDenom and EthIBCDenom is hard coded.
They are used to determine the value of MaxSwapAmount and are used in TradeExactInputForOutput and TradeInputForExactOutput.

Here's how to get the value of MaxSwapAmount

31:        DefaultMaxSwapAmount          = sdk.NewCoins(
32:                sdk.NewCoin(UsdcIBCDenom, sdk.NewIntWithDecimal(10, 6)),
33:                sdk.NewCoin(UsdtIBCDenom, sdk.NewIntWithDecimal(10, 6)),
34:                sdk.NewCoin(EthIBCDenom, sdk.NewIntWithDecimal(1, 17)),

sdk.NewCoin returns a new coin with a denomination and amount. It will panic if the amount is negative or if the denomination is invalid.

How are IBC denoms derived? . Since UsdcIBCDenom, UsdtIBCDenom and EthIBCDenom are hard coded into hashes. The trade-off when using a hash is that you cannot compute the input given the output (hashing is an irreversible operation). Therefore, the ICS-20 module keeps a mapping of IBC denominations it has encountered in order to look up the original path and base_denom
```
// hash() representing a SHA256 hashing function returning a string
ibc_denom := 'ibc/' + hash('path' + 'base_denom')
```
This example let you now know that there is an IBC port transfer and channel that corresponds to the IBC connection
These values may change in the future as the project is still in development, which in turn leads to the IBC demon being unusable and leading to panic.

Tools Used

Manual review

Recommended Mitigation Steps

IBC demon should not be hardcoded at this time. Consider redeploying and possibly updating the value in the future..

Assessed type

Other

c4-pre-sort commented 1 year ago

JeffCX marked the issue as low quality report

JeffCX commented 1 year ago

Don't see the hardcoded Denom produce issue, at most QA

c4-judge commented 1 year ago

0xean changed the severity to QA (Quality Assurance)