Making transfer v2 channel unupgradable through the forwarding
Low
C
Cosmos
Submitted None
Actions:
Reported by
unknown_feature
Vulnerability Details
Technical details and impact analysis
### Summary of Impact
On one hand we have this upgrade functionality that has this specific check [HasInflightPackets](https://github.com/cosmos/ibc-go/blob/e10bbbc47fb70e138a3369902d18586b3da95800/modules/core/04-channel/keeper/keeper.go#L712). It looks up channel's committed packets. And if there are any it won't set the channel into `FLUSHCOMPLETE`. In [handleFlushState](https://github.com/cosmos/ibc-go/blob/f5e1a4c33c01d355323ea368d8d63664180736e5/modules/core/04-channel/keeper/packet.go#L486), in [WriteUpgradeAckChannel](https://github.com/cosmos/ibc-go/blob/f5e1a4c33c01d355323ea368d8d63664180736e5/modules/core/04-channel/keeper/upgrade.go#L354) and in [WriteUpgradeConfirmChannel](https://github.com/cosmos/ibc-go/blob/f5e1a4c33c01d355323ea368d8d63664180736e5/modules/core/04-channel/keeper/upgrade.go#L484). And upgrade can't be fully completed if the channel is not in the `FLUSHCOMPLETE` state. In [ChanUpgradeOpen](https://github.com/cosmos/ibc-go/blob/f5e1a4c33c01d355323ea368d8d63664180736e5/modules/core/04-channel/keeper/upgrade.go#L512). Which would error and prevent it from getting into [WriteUpgradeOpenChannel](https://github.com/cosmos/ibc-go/blob/f5e1a4c33c01d355323ea368d8d63664180736e5/modules/core/keeper/msg_server.go#L844) where it sets the channel back to open with [upgraded fields](https://github.com/cosmos/ibc-go/blob/f5e1a4c33c01d355323ea368d8d63664180736e5/modules/core/04-channel/keeper/upgrade.go#L634)
Packet commitments can be deleted through 2 ways: [acknowledgement](https://github.com/cosmos/ibc-go/blob/f5e1a4c33c01d355323ea368d8d63664180736e5/modules/core/04-channel/keeper/packet.go#L445) and [timeout](https://github.com/cosmos/ibc-go/blob/f5e1a4c33c01d355323ea368d8d63664180736e5/modules/core/04-channel/keeper/timeout.go#L138).
On the other hand we have this forwarding [functionality](https://github.com/cosmos/ibc-go/blob/0e72d08c4aeded65f74d93500fb282314fd1acc0/modules/apps/transfer/ibc_module.go#L203) that does NOT write an acknowledgement for the [forwarded packet](https://github.com/cosmos/ibc-go/blob/f5e1a4c33c01d355323ea368d8d63664180736e5/modules/core/keeper/msg_server.go#L412) until it [receives](https://github.com/cosmos/ibc-go/blob/0e72d08c4aeded65f74d93500fb282314fd1acc0/modules/apps/transfer/keeper/relay.go#L287) an acknowledgement to the channel it forwarded the packet to.
So this approach to forwarding introduced a situation when a legit channel might become dependent on a malicious channel. And malicious channel doesn't have to acknowledge packets. And if it doesn't ack them, it seems like the legit channel will not be able to transition into `FLUSHCOMPLETE` state. And as a result would not be able to complete upgrades. Neither current nor future.
### Steps to Reproduce
The POC first creates legit transfer v2 channels: channel-0 on ibc-0 <-> channel-0 on ibc-1. Thst's the part which relayer takes care of.
Then in my app I open another 2 channels channel-1 on ibc-0 <-my local chain ibc-1 || my local chain ibc-0 -> channel-1 on ibc-1.
And I submit receive packet that looks like [this](https://gist.github.com/unknownfeature/b5c7323df65300a4699a3fa5471e9153#file-gistfile1-txt-L87):
```
ibc0Cw.ReceivePacketV2(ibc0Path, types.FungibleTokenPacketDataV2{
Tokens: []types.Token{
{
Amount: "1",
Denom: types.Denom{Base: "x"},
},
},
Memo: "",
Sender: ibc1Cw.Address(),
Receiver: ibc0Cw.Address(),
Forwarding: types.ForwardingPacketData{Hops: []types.Hop{
{ChannelId: legitChannel, PortId: port}, {ChannelId: ibc0Path.Dest.ChanId, PortId: port},
}, DestinationMemo: ""},
})
```
Packet is getting received on channel-1 on ibc-0, because of the forwarding it gets submitted via channel-0 to channel-0 on ibc-1 where it gets forwarded again to my channel-1 on ibc-1. And because of that it doesn't write any acknowledgements. Writes the receipt though. Which means that we won't be able to timeout it. So we end up in a situation when a legit channel-0 on ibc-0 will continues having committed (inflight) packets until my channel-1 on ibc-1 receives an acknowledgement. Which it won't. So the victim channel is channel-0 on ibc-0. Which will end up having this commitment.
So the POC itself is not a new scenario. It just shows the scenario that can create these everlasting commitments on legit channels.
Methods that I use in the POC are defined here https://github.com/h1uf/ibc-tools/blob/master/chain/wrapper.go,
(make sure no simd is running)
1. Have Go installed
2. Check out and build https://github.com/cosmos/ibc-go. Run `make build` and take a note of the path to simd
2. Download my relayer https://github.com/h1uf/relayer. It's pretty much the same as original just demo scripts were edited. You will need to run a relayer. You can build that one or a legit one. And make sure rly is in the path. Just scripts need to be ones form the https://github.com/h1uf/relayer/tree/main/examples/demo folder. Replace [path to binary](https://github.com/h1uf/relayer/blob/main/examples/demo/scripts/two-chainz#L8) with the path to simd that you noted in a prev step. Run `./dev-env`
3. Meanwhile checkout https://github.com/h1uf/ibc-tools. And put attached `lock_upgrade.go` to the root. Make sure your [path](https://gist.github.com/unknownfeature/b5c7323df65300a4699a3fa5471e9153#file-gistfile1-txt-L22) points to correct data folder and your [home](https://gist.github.com/unknownfeature/b5c7323df65300a4699a3fa5471e9153#file-gistfile1-txt-L30) path is your current dir. It will be parsing seed phrase from the chain data and storing keys in cur dir.
4. Run the app
It establishes channel-1 to channel-1 via my proxy chain. So everything before this line https://gist.github.com/unknownfeature/b5c7323df65300a4699a3fa5471e9153#file-gistfile1-txt-L79 is just establishing a channel. And then receive the packet on channel-1 on ibc-0 . If you switch to relayer you will start seeing that there is no ack in a few minutes. But that's all expected. Just not expected(as it seems) that my channel will not write acknowledgments. It doesn't seem like it's expected that commitments can stay committed forever(unless I missed a place where they can be deleted except 2 places that I've mentioned). It seems rather unacknowledged that this dependency on potentially malicious channel got introduced through this part of forwarding functionality. Where it waits for an ack from another channel.
### Workarounds
From what I see it doesn't seem like. There is no other way to delete commitments and it doesn't seem the channel can become FLUSHCOMPLETE w\o flushing all the packets. Though I could miss something.
### Supporting Material/References
- lock_upgrade.go - the POC
- upgrade.mov - the video of the POC
## Impact
Transfer v2 channels could be made unupgradable through the forwarding functionality that allows malicious channels to create commitments on legit channels that will be left unacknowledged.
Report Details
Additional information and metadata
State
Closed
Substate
Resolved
Submitted
Weakness
Business Logic Errors