Replacing ICA active channel during the upgrade and a bit more

### Summary of Impact There are 2 potential issues here. 1. Active channel is set on controller during the channel [ack](https://github.com/cosmos/ibc-go/blob/524ff0d65a3874d0264dfed6ff1ad90d104ee9a0/modules/apps/27-interchain-accounts/controller/keeper/handshake.go#L123). And it's a [check than act](https://github.com/cosmos/ibc-go/blob/524ff0d65a3874d0264dfed6ff1ad90d104ee9a0/modules/apps/27-interchain-accounts/controller/keeper/handshake.go#L106) operation , and it's atomic. But [GetOpenActiveChannel](https://github.com/cosmos/ibc-go/blob/8575743db893ee8e3bae5cbdab553b660b6844b5/modules/apps/27-interchain-accounts/controller/keeper/keeper.go#L133) only returns the channel if it's open. **Not when it's flushing**. 2. Active channel is set on host during the channel [open confirm](https://github.com/cosmos/ibc-go/blob/7aae649697521e886e161832f9be87d3907a93f6/modules/apps/27-interchain-accounts/host/keeper/handshake.go#L54) but the check if the channel exists is done in [try](https://github.com/cosmos/ibc-go/blob/7aae649697521e886e161832f9be87d3907a93f6/modules/apps/27-interchain-accounts/host/keeper/handshake.go#L54). It does include FLUSH* states but **it's not atomic**. ### Steps to Reproduce We monitor newly inited channels on the [source chain](https://gist.github.com/unknownfeature/45d0ad6cd00f8f70ff59e6efe8741cd8#file-main-go-L244). We iterate over [returned channels](https://gist.github.com/unknownfeature/45d0ad6cd00f8f70ff59e6efe8741cd8#file-main-go-L66) and run the attack for a matched one. We submit ICA creation once everything is up(see the video) and [this place](https://github.com/h1uf/relayer/blob/ica/examples/demo/script.sh#L12) is responsible for performing victim's part. I used version [as is](https://gist.github.com/unknownfeature/45d0ad6cd00f8f70ff59e6efe8741cd8#file-main-go-L230) because it would change during the upgrade. And the main idea to have this new channel different from the legit channel. The attack consists of 2 steps: 1. We do the preparation work. Because check than act operation on the host is not atomic we have a window of opportunity between ICA channel init on the controller and ICA channel open confirm on the host. And during that window we'll do [chan init](https://gist.github.com/unknownfeature/45d0ad6cd00f8f70ff59e6efe8741cd8#file-main-go-L103) with the ICA port and connection on the controller and [chan try](https://gist.github.com/unknownfeature/45d0ad6cd00f8f70ff59e6efe8741cd8#file-main-go-L114) on the host. Then we wait for the victim channel to start upgrading. The [script](https://github.com/h1uf/relayer/blob/ica/examples/demo/script.sh#L20) will initiate upgrade proposal for a new encoding. But our channel will have an old encoding. Encoding is needed to deserialize the [tx](https://github.com/cosmos/ibc-go/blob/7aae649697521e886e161832f9be87d3907a93f6/modules/apps/27-interchain-accounts/host/keeper/relay.go#L36) on the host. 2. Once the proposal passes through, the attack picks up new upgrade sequence and performs the whole channel update. And in parallel it completes the handshake for the malicious channel. Legit channel: `channel-1`, my channel `channel-2` I run relayer in parallel too because I need ICA channel to complete like it would do in real world. But also the relaye picks up mu channels. I stop it after legit channel is open but it prints the following message during the handshke: ``` error Error sending messages {"path_name": "demo", "src_chain_id": "ibc-1", "dst_chain_id": "ibc-0", "src_client_id": "07-tendermint-0", "dst_client_id": "07-tendermint-0", "error": "rpc error: code = Unknown desc = rpc error: code = Unknown desc = failed to execute message; message index: 1: channel open ack callback failed for port ID: icacontroller-cosmos1zr776t6fpjtyx8vqfhn0ene3jzm5q7nc2674d0, channel ID: channel-2: existing active channel channel-1 for portID icacontroller-cosmos1zr776t6fpjtyx8vqfhn0ene3jzm5q7nc2674d0: active channel already set for this owner [``` Which is correct. The error comes exactly from [here](https://github.com/cosmos/ibc-go/blob/524ff0d65a3874d0264dfed6ff1ad90d104ee9a0/modules/apps/27-interchain-accounts/controller/keeper/handshake.go#L107) But after once upgrade is started, I complete the handshake it passes: ``` channel acked successfully acked channe on the source chainl channel-2 for port icacontroller-cosmos1zr776t6fpjtyx8vqfhn0ene3jzm5q7nc2674d0 and connection connection-0 {"level":"info","msg":"Successful transaction","chain_id":"ibc-0","gas_used":105368,"fees":"3310stake","fee_payer":"\u001bZf\u0007\u00144u \ufffd\ufffd\ufffdx\ufffdh\ufffd\ufffdT\ufffdM\u0012","height":134,"msg_types":["/ibc.core.channel.v1.MsgChannelOpenAck"],"tx_hash":"1E1968F24B19D38D0C1ABA0212884E6D295796660862F99C249002739548EE40"} u ``` Then it passes open confirm on the host too. And we set a new channel that has different settings as active for this ICA. (have Go installed, and make sure no simd running) 1. I had to make an [adjustment](https://github.com/cosmos/ibc-go/compare/main...h1uf:ibc-go:main) in ibc-go in order to submiit a proposal for upgrade. Took the logic from [wasm light client](https://github.com/cosmos/ibc-go/blob/8575743db893ee8e3bae5cbdab553b660b6844b5/modules/light-clients/08-wasm/client/cli/tx.go#L51). I was getting an eror that the proposal has to be signed by the gov module(which makes sense). So not sure if that's a bug or authority account has to be set up. Either way either check out my [modified fork](https://github.com/h1uf/ibc-go)or set up your account to be a gov authority if possible. So you would be able to submit the upgrade. And build ibc-go `make build`. Take a note of the path to simd (in build folder) and change it [here](https://github.com/h1uf/relayer/blob/ica/examples/demo/dev-env#L5) (Also just a note that our malicious channel won't be affected by the upgrade because it's not open. ) 2. Checkout [my relayer ica branch](https://github.com/h1uf/relayer/tree/ica). Only demo folder is needed for this attack. But also a rly binary in path which you can build from the official relayer or my relayer. 3. Checkout https://github.com/unknownfeature/ibc-tools/tree/one, switch to branch `one` and put attached main.go to the root. 4. Modify [rootFolder](https://gist.github.com/unknownfeature/45d0ad6cd00f8f70ff59e6efe8741cd8#file-main-go-L190) in the main.go to point to the correct folder with the scripts. 5. Go to the relayer/examples/demo(rootFolder) and run ./dev-env. And wait until you see this message: "Run 'rly start and run active_channel.go, press ENTER after everything is up" 6. Run main.go, it will print 2 lines and will be waiting. After that open another tab and start `rly start`. We need it to finish the ICA handshake. But also it will be trying to finish the handshake we initiated in the attack which will error at first but then once we pass the ack it will be trying to continue our handshake. Which will accomplish the same thing as us but just my little attack might error, and it won't look good for the demo purposes. But the result of the attack will be the same even if the relayer finishes it. So I just wait for it to open the legitimate channel and stop it after it prints " Successfully created new channel {"chain_name": **"ibc-1",** ". Should be on ibc-1 which means that the handshake is over. 7. Once everything is up hit 'ENTER" where dev-env requested it. And look at the output of the attack. It will take some time. I set proposal waiting period to [30s](https://github.com/h1uf/relayer/blob/ica/examples/demo/scripts/one-chain#L124). But also I wait for the ICA channels for 20s. I also wanted to say that sometimes my chan ack arrived to the source chain faster than the legit one. That's why I [sleep here](https://gist.github.com/unknownfeature/45d0ad6cd00f8f70ff59e6efe8741cd8#file-main-go-L102). But it's an interesting use case as well. In some cases active channel will be set to a wrong one during the handshake. Very opportunistic. And who knows how it'll be acting in real life. ### Workarounds Doesn't look like ### Supporting Material/References * active_channel_upgrade.mov - video of the POC * main.go - the POC ## Impact The app gives opportunities to replace established active channel with the malicious one during the upgrade (or sometimes set a wrong active channel during the handshake) on the ICA. We can't change ICA address, we can't change connection. The only thing we can change is encoding. Which can prevent host chain from deserializing ICA transactions. So it will need another upgrade to be able to work with it. Plus it will require some investigation taking into account that there is no query method to see what's the active channel. So it will take some time to figure it out and make it work again. Plus we can pre setup not one but several channels. And use each for each upgrade.