Interchain Accounts: possible DOS attacks via port binding

[andrey-kuprianov]

Surfaced from @informalsystems audit of ICS27 InterchainAccounts at hash b8bc1a8

The creation of an interchain account starts in the function InitInterchainAccount():

func (k Keeper) InitInterchainAccount(ctx sdk.Context, connectionID, counterpartyConnectionID, owner string) error {
	portID, err := types.GeneratePortID(owner, connectionID, counterpartyConnectionID)
	if err != nil {
		return err
	}
	if k.portKeeper.IsBound(ctx, portID) {
		return sdkerrors.Wrap(types.ErrPortAlreadyBound, portID)
	}
    ...

The function GeneratePortID generates the port identifier in the following format:

ics27-1.{connection-id}.{counterparty-connection-id}.{owner-address}

As can be seen from the above fragment, interchain account creation will fail if the port is already bound. But according to the ICS05 (port allocation), we have:

• Ports are allocated first-come first-serve
• Once a module has bound to a port, no other modules can use that port until the module releases it

Combined, this creates a possibility of DOS attacks on Interchain Accounts:

• ICA module doesn't operate in a vacuum; there are other modules;
• the port identifier is too predictable: connection ids are essentially constant, and the owner address is known in advance for many accounts;
• thus, any other module, besides ICA, can easily bind any number of ICA ports, preventing it from functioning correctly.

Note: the problem has been surfaced via formal modeling of Interchain Accounts in TLA+: see the ICA_0 TLA+ model.

Recommendation

Change the port identifier format to include some kind of a random seed, generated upon port creation, e.g. like that:

ics27-1.{connection-id}.{counterparty-connection-id}.{owner-address}.{seed}

This will have an additional benefit of allowing an account owner on a controller chain to create multiple interchain accounts on a host chain for the same owner address, which may be used e.g. for differentiating interchain accounts for incompatible purposes, without forcing the user to create separate accounts on the controller chain.

---

For Admin Use

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

[colin-axner]

Thanks @andrey-kuprianov!

Maybe instead of a seed we can attach the controller channelID (it would use the next available channel sequence since the channel is created after the port is binded, but in the same state transition so it'll be atomic)? But this would complicate the existing active channel design

The reason why I think a seed is unnecessary is because the SDK currently functions assuming all modules are honest. They may be faulty, but we can assume there is no module trying to DoS interchain accounts since they could simply access the bank keeper without any issues ie the SDK doesn't protect against malicious modules

Appending the channelID is useful because then it allows multiple controller modules on the controller chain to use the same account addresses without collision

This will have an additional benefit of allowing an account owner on a controller chain to create multiple interchain accounts on a host chain for the same owner address

This is interesting. Is there a benefit to the host chain allowing for multiple accounts associated with a single address as opposed to the controller chain mapping one form of authentication for multiple addresses ie mapping a public key to many addresses via a nonce?

[andrey-kuprianov]

@colin-axner thanks for the write-up!

I understand the assumption that the modules are honest. But we should consider also a scenario not of a malicious module being added to IBC, but of a honest module that might be abused for attack purposes. Imagine this scenario:

• a module AwesomeWallet is added to IBC in the future. The module allows users to register a named wallet, and to perform operations on it from other chains connected via IBC.
• the developers of this module see that IBC enforces the first-come-first-serve discipline wrt. port names.
• they think: "Aha! we can employ this functionality to make our life easier. Whenever someone registers an account in AwesomeWallet, we register the port with the same name; this allows us to easily track which accounts are already taken, and accept connections to the wallet via the port with the same name"

What does addition of AwesomeWallet means for Interchain Accounts? Right, the developers of AwesomeWallet unintentionally created a security vulnerability: anyone can register an awesome wallet with the ICA-specific name, thus preventing ICA from functioning.

If you weigh the two alternatives:

• add a seed to ICA port names
• require all future IBC modules to take into account security of ICA when registering their ports

I believe the first alternative is much more secure and bullet-proof.

[andrey-kuprianov]

This is interesting. Is there a benefit to the host chain allowing for multiple accounts associated with a single address as opposed to the controller chain mapping one form of authentication for multiple addresses ie mapping a public key to many addresses via a nonce?

Could you please clarify this? Sorry, don't quite get what you meant:)

[colin-axner]

Thanks for the further clarification @andrey-kuprianov!

In this situation, any IBC application module which binds to ports dynamically is affected by this byzantine module. Rather than always using a seed, I think the longer term fix would be to allow IBC applications to claim a port prefix. This would allow only the interchain accounts controller module to bind to ports under ics27

In the short term, I think it is fine to release interchain accounts with this DoS edge case possible. It seems reasonable to expect IBC application modules which allow user inputted ports to be prefixed as a security precaution

Could you please clarify this?

Yup. So lets say we have a private key, pk, on the controller chain. The controller chain could generate multiple addresses from this pk (via a seed). That is, I use my pk to authenticate transactions on the controller chain and the controller chain allows me to derive multiple addresses from this pk (binding multiple ports). If our pk is really a governance proposal, the governance proposal could provide a seed used to derive the address

The other alternative is deriving multiple addresses on the host chain. So instead of the controller chain generating multiple addresses from a single pk, the host chain is doing this. My personal opinion is that it is cleaner from an architecture point of view to allow the controller chain to do this address derivation, as it reduces overall complexity in the ics specification. Does this make sense?

[andrey-kuprianov]

Rather than always using a seed, I think the longer term fix would be to allow IBC applications to claim a port prefix.

Totally support this! It is indeed a stable long-term solution to the problem.

Wrt address derivation: now I see what you mean. I think my confusion stems from the fact is that ICS27 outlines only part of the design, leaving the authentication module beyond the scene.

Probably you are right, and deriving multiple addresses on the controller chain is a fine and better solution than deriving them on the host chain. But this whole business with ports still makes me uneasy, to be honest... I will think about it more, and return to this later.

[colin-axner]

We will not be addressing this issue in the first release of interchain accounts. We will add documentation and in a future release provide a long term fix that can be reused for all IBC applications via port prefixing.