Host chain allows overwriting of the active channel

[colin-axner]

Summary

The host chain assumes the controller chain will never allow a channel to OPEN if it is not in the active channel mapping.

Problem Definition

OnChanOpenConfirm will overwrite an active channel. This is fine for creating a new active channel after the previous one closed, but if the controller chain allows two channels to be opened for the same port (by incorrectly accounting for active channels), then this will overwrite an existing active channel

Proposal

Be defensive. If an active channel exist, return an error. Please add a test case for this scenario

---

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged/assigned

[colin-axner]

@AdityaSripal I think this should be updated in the spec

[AdityaSripal]

But if the controller chain rewrites the active channel, shouldn't the host chain just use the latest active channel? I fail to see an attack here

[seantking]

The host chain assumes the controller chain will never allow a channel to OPEN if it is not in the active channel mapping.

I don't understand this point. The current architecture will allow a controller chain to initialize a new channel as many times as it likes. I don't understand why this would be a security issue.

The only potential vulnerability I see here is in the case where a relayer could spam channel creation after the first InitInterchainAccount and potentially stop packets from reaching the interchain account (by overwriting the active channel by spamming channel creation on the bound port for a specific interchain account).

[colin-axner]

But if the controller chain rewrites the active channel, shouldn't the host chain just use the latest active channel? I fail to see an attack here

There's a race condition here though right? If chainA ACKs channel-1 and channel-2 on the same port. A relayer could decide which becomes the active channel on chainB by submitting the OpenConfirm in a certain order. We could end up in a state where the controller active channel does not equal the host active channel (and it cannot be fixed)

I think the host chain should defensively consider overwrites of an active channel on the controller chain as byzantine behaviour (not necessarily attacking, but something is going wrong)

[colin-axner]

Action items:

• Ensure only 1 active channel can be created on ACK by checking if active channel exists + and is open
• document that host (in OpenConfirm) assumes controller only allows 1 channel to be ACK'd
• ensure specs have this check
• do defensive check in OpenTry