Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: Chan Blocker can keep chains from hooking up #465

Closed
ethanfrey opened this issue Aug 10, 2020 · 7 comments · Fixed by #482
Closed

Bug: Chan Blocker can keep chains from hooking up #465

ethanfrey opened this issue Aug 10, 2020 · 7 comments · Fixed by #482

Comments

@ethanfrey
Copy link
Contributor

After some nice discussion with @cwgoes on CosmWasm/wasmd#214 I finally understand the details of the IBC handshakes, and some key components work differently than I expected. One is the channel versioning (which I assumed the counterparty selects), but that is already addressed in ADR 025, which I hope is (or soon will be) supported by the cosmos sdk.

However, there is another more critical issue that can keep two chains from ever hooking up completely and establishing an ics20 channel. It requires a malicious client/relayer, but I do not think there is a clear way to stop this on the server side. Below, I document the "Chan Blocker" protocol. Here we have Alice and Bob as two chains that have an open connection. Randy is an honest relayer that wants to hook up an ICS20 channel. And Chad is a malicious "Chan Blocker" that wants to keep this from happening. Note that neither Randy nor Chad have any special permissions on either chain.

  1. Randy submits an ChanOpenInit transaction to Alice. He sets the version to transfer(?) and the local and remote port to transfer. He then selects channelIDs for both sides. These can be any ids, for now, let's call them bob on Alice and alice on Bob (which would be sensible names, but any other name works).
  2. The transaction is included in block Alice:H (height H on Alice). Randy now waits for block Alice:H+1, so he can get a valid AppHash to prove the packet.
  3. Chad sees the transaction in block Alice:H and acts quickly. He runs over to Bob and tells him Alice is going to a party at his house. Or more technically, Chad submits a ChanOpenInit transaction to Bob. He properly sets the version to transfer(?) and the local and remote port to transfer. However, he wants to claim the channelID alice on chain Bob, so he sets the channels to Alice:chad and Bob:alice.
  4. When Randy dutifully relays his original message to bob, he finds the channel Bob:transfer/alice is already claimed and the OnChanOpenTry fails. Randy sadly goes home.

Note that during the attack Chad opened up a new possibility for Randy. He just needs to properly relay the Bob:transfer/alice -> Alice:transfer/chad init message over to alice. Of course, the every malicious Chad will run over to block that one before it can be done.

You would think this requires Chad to run around constantly always blocking the next step of his last action. But you could imagine a set of Inits that would block full circle, so Chad can rest until Randy starts a new ChanOpenInit.

  1. Randy: init Alice:transfer/bob -> Bob:transfer/alice
  2. Chad init Bob:transfer/alice -> Bob:transfer/chad (blocks the first init above, but opens up a channel)
  3. Chad: init Alice:transfer/chad -> Bob:transfer/foo (blocks the second one, but opens up a channel)
  4. Chad: init Bob:transfer/foo -> Alice:transfer/bob (blocks the third one, and re-uses a blocked channel).

At the end, channels bob and chad are claimed on Alice and alice and foo are claimed on Bob, and none of the above init messages can succeed.

Note that this is not even a race condition, as Chad always has a 1 block head-start. As soon as block H is formed, he can submt the next transaction (or even submit 2, 3, 4 simulataneously when #1 is in a block). Randy must wait a full block (until block H+1 is formed with the AppHash) to continue the OnChanOpenTry for the initial request.


Proposal: Similar to the connection issue #459 and the proposed solution #462 (let the recipient set connectionID and version), I propose that we allow the recipient chain to select the RemoteChannelID, instead of the initiating chain.

I don't see a security hole here, as if Alice initiates Alice:transfer/bob -> Bob:transfer/? and later receives an valid ChanOpenAck from bob of Bob:transfer/XYZ -> Alice:transfer/bob this must be valid regardless of what XYZ is (that was committed on Bob). I see no way a malicious Chad could forge such a connection, and it allows Bob to select any available channel ID when the requests is delivered.

@cwgoes
Copy link
Contributor

cwgoes commented Aug 10, 2020

Yes; this race condition is known, but there isn't a clear advantage to be gained, since Chad has to pay transaction fees and Randy can just create a new channel with a different identifier. I agree that allowing the receiving chain to choose a channel identifier is helpful, though.

@ethanfrey
Copy link
Contributor Author

Assume Chad is a shill from another chain that wants to prove how much Cosmos sucks.

We can submit 3 tx per block (5s) to keep two chains from connecting, or about 51840tx/day. These don't need header proofs, so maybe 100k gas? Which would be 2500uatom/tx at present rates. Thus if we have a relayer who tries to make a new channel between A and B every block, one could spend 3x that price (or ~130 atoms/day) to keep the chains apart. Even at 5 euros/atom, that would be maybe 20k euros/month, which is far much less than such a chain (and IBC in general) would loose in credibility.

@ethanfrey
Copy link
Contributor Author

I agree that allowing the receiving chain to choose a channel identifier is helpful, though.

Is there any reason NOT to have this be the default behavior? Why was it chosen the other way around?

@cwgoes
Copy link
Contributor

cwgoes commented Aug 10, 2020

I agree that allowing the receiving chain to choose a channel identifier is helpful, though.

Is there any reason NOT to have this be the default behavior? Why was it chosen the other way around?

It was initially the other way around because identifiers may have semantic significance, e.g they could be used to determine what protocol would be spoken, or a module could multiplex other submodules (perhaps contracts) by using subsets of the channel identifier range.

@cwgoes
Copy link
Contributor

cwgoes commented Aug 10, 2020

Assume Chad is a shill from another chain that wants to prove how much Cosmos sucks.

We can submit 3 tx per block (5s) to keep two chains from connecting, or about 51840tx/day. These don't need header proofs, so maybe 100k gas? Which would be 2500uatom/tx at present rates. Thus if we have a relayer who tries to make a new channel between A and B every block, one could spend 3x that price (or ~130 atoms/day) to keep the chains apart. Even at 5 euros/atom, that would be maybe 20k euros/month, which is far much less than such a chain (and IBC in general) would loose in credibility.

Sure, but different relayers can try different channel identifiers, the spend required to DoS will always end up being proportional.

@ethanfrey
Copy link
Contributor Author

It was initially the other way around because identifiers may have semantic significance, e.g they could be used to determine what protocol would be spoken

Okay, this is where we use the Version field now?

or a module could multiplex other submodules (perhaps contracts) by using subsets of the channel identifier range.

Ahh.. interesting idea. I wish some of this reasoning was in the ICS spec. I see the design benefit here, as we have to bind one port per contract in wasmd, this would be a nice way to multiplex on one port. However, I would still stick with our design now, but others may want this choice.

@cwgoes
Copy link
Contributor

cwgoes commented Aug 10, 2020

Okay, this is where we use the Version field now?

Yes; if we add the ability to require a fixed version at the start of the handshake (also in #462) that should cover protocol negotiation.

Ahh.. interesting idea. I wish some of this reasoning was in the ICS spec. I see the design benefit here, as we have to bind one port per contract in wasmd, this would be a nice way to multiplex on one port. However, I would still stick with our design now, but others may want this choice.

Good point, we should mention this explicitly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants