fix: apply 4992

[snobbee]

Description

Back-porting of security fix in #4992 into v4.5.x

Commit Message / Changelog Entry

fix: apply 4992

see the guidelines for commit messages. (view raw markdown for examples)

---

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

• Targeted PR against correct branch (see CONTRIBUTING.md).
• Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
• Code follows the module structure standards and Go style guide.
• Wrote unit and integration tests.
• Updated relevant documentation (docs/) or specification (x/<module>/spec/).
• Added relevant godoc comments.
• Provide a commit message to be used for the changelog entry in the PR description for review.
• Re-reviewed Files changed in the Github PR explorer.
• Review Codecov Report in the comment section below once CI passes.

[crodriguezvega]

@snobbee May I ask why you would need to fix back ported to v4.5.x? The extra length checking we added does not actually solve the root cause of the problem, as an attacker could create spam transactions by other means. Also this problem has not been categorised as a security vulnerability (it seems to be more of a mempool/protocol design issue), and given that v4.5. has actually reached end of life, we believe that we should not backport and release any improvements in that release line.

[snobbee]

@snobbee May I ask why you would need to fix back ported to v4.5.x? The extra length checking we added does not actually solve the root cause of the problem, as an attacker could create spam transactions by other means. Also this problem has not been categorised as a security vulnerability (it seems to be more of a mempool/protocol design issue), and given that v4.5. has actually reached end of life, we believe that we should not backport and release any improvements in that release line.

@crodriguezvega I understand the points raised regarding the backporting to v4.5.x, and I agree that the extra length checking doesn't address the root cause of the issue. However, there are currently active chains, such as Osmosis and Sifchain, that are operating on v4.5.1.

These chains continue to experience the impact of spam attacks due to the lack of verification in the memo field. While it's true that attackers might use other methods, the proposed fix could mitigate the current known attack vector significantly.

By patching this within the ibc-go repository for the v4.5.x versions, we could provide these chains with an immediate improvement to their resilience against such attacks. It would serve as a stopgap solution until they can transition to a more robust solution that solved the issue at its core, when it gets available.

Given the circumstances, I believe it is beneficial to backport this fix, thus extending support to any other chains that still depend on this version. This could be a valuable measure to ensure the stability and security of the broader ecosystem during the transition period.

Would it be possible to reconsider the decision in light of this context?

[crodriguezvega]

@snobbee Thank you for your detailed reply and for giving your point of view. Unfortunately we discussed this internally and we do not think that this fix warrants to be back ported to a version that has reached End of Life. Please note that the EoL date for the v4 line had already been extended. Besides, this fix is state-machine breaking, so we actually cannot back port it to v4.5.x. And this also means that chains would require a coordinated upgrade.

As an alternative, wouldn't some of the mitigations recommended here be enough?

I will close the PR now.