`packet.DestinationChannel` IS CHECKED AGAINST THE `WhitelistedChannels`, BUT `packet.SourceChannel` SHOULD BE CHECKED INSTEAD, AS PER THE PROTOCOL DESIGN REQUIREMENTS

[code423n4]

Lines of code

https://github.com/code-423n4/2023-06-canto/blob/main/Canto/x/onboarding/keeper/ibc_callbacks.go#L44-L50

Vulnerability details

Impact

In the ibc_callbacks.OnRecvPacket function, the Source Channel of the transferred packet is required to be checked against the WhitelistedChannels of the module. If the Source Channel of the packet is not in the WhitelistedChannels list then the auto swap and convert will not be triggered.

The following code snippet executes this logic in the ibc_callbacks.OnRecvPacket function.

// check source channel is in the whitelist channels
var found bool
for _, s := range params.WhitelistedChannels { //@audit-info - get the whitelistedchannels from the parameters
    if s == packet.DestinationChannel { //@audit-issue - comment says check the source channel of teh packet but here the destination channel is checked?
        found = true
    }
}

But the issue here is that the Desitnation Channel packet.DestinationChannel is checked here against the values of the WhitelistedChannels.

Hence if the Destination Channel of the Canto blockchain module is whitelisted in the WhitelistedChannels list then every token transfer coming from any Source Channel will be eligible for the auto swap and convert.

Hence the purpose of Whitelisting Source Channels is not properly implemented here. Thus the implementation of the logic is not in line with the protocol design requirement.

Proof of Concept

// check source channel is in the whitelist channels
var found bool
for _, s := range params.WhitelistedChannels {
    if s == packet.DestinationChannel {
        found = true
    }
}

https://github.com/code-423n4/2023-06-canto/blob/main/Canto/x/onboarding/keeper/ibc_callbacks.go#L44-L50

Tools Used

VSCode and Manual Review

Recommended Mitigation Steps

It is recommended to check the packet.SourceChannel against the WhitelistedChannels as shown in the modified code snippet below:

// check source channel is in the whitelist channels
var found bool
for _, s := range params.WhitelistedChannels { //@audit-info - get the whitelistedchannels from the parameters
    if s == packet.SourceChannel { //@audit-issue - comment says check the source channel of teh packet but here the destination channel is checked?
        found = true
    }
}

Assessed type

Other

[c4-pre-sort]

JeffCX marked the issue as primary issue

[tkkwon1998]

The "source channel" and "destination channel" labels are slight misnomers from IBC itself, which leads to a lot of confusion. The destination channel just means the channel on the receiving chain. That is the channel that is used to both receive and send IBC packets to another channel.

A better way to think about this would just to call it "gravity-bridge-channel", because all assets coming from Gravity Bridge will go through this channel, and all assets sent to Gravity Bridge will also go through this same channel.

[c4-sponsor]

tkkwon1998 marked the issue as sponsor disputed

[0xean]

@tkkwon1998 - thanks for the response here, do you have any docs that I could take a look at to verify this before I close it?

[tkkwon1998]

@0xean here's docs for IBC that go over channels: https://docs.cosmos.network/v0.45/ibc/overview.html

[0xean]

Closing as invalid. none of the wardens submitted sufficient proof of there findings. From all of the documentation (which isn't great) the implementation does seems to be correct.

[c4-judge]

0xean marked the issue as unsatisfactory: Invalid