ICS2: maximum length of client type in client identifier is not specified and can be abused

[odeke-em]

The specs do define the character set of the client identifier as ASCII per however, they don't specify a maximum length I discovered this while auditing ibc-go per https://github.com/cosmos/ibc-go/issues/2269 that the length of client identifiers is unspecified and can be abused to say even >=1GB. We need to have some sane limit that'll cover as many use cases and avoid security issues and attacks.

Kindly cc-ing @zmanian @crodriguezvega

[crodriguezvega]

Hi @odeke-em, isn't the length specification right below the highlighted text on the screenshot you attached in the issue?

[odeke-em]

Thank you @crodriguezvega for pointing this out! I did see those limits and when I asked in a security channel if those were the limits to be accepted, I got told that chains were allowed to have as long identifiers (not just about enforcement, but perhaps not yet agreed upon by everyone) which made me come here and file this issue

[crodriguezvega]

In ibc-go we have validation to make sure the identifiers are in the length range specified in the spec.

I think no changes are required in the spec, so the issue can be closed. Do you agree, @odeke-em?

[crodriguezvega]

Closing this issue for now, since it doesn't seem like any changes are required at the moment in the spec.