Conversation
* deps: removed local pin of ibc-go * deps(e2e): ran go mod tidy
…8742) * imp: add extra validation to ica msgs (cosmos#8734) * imp: add extra validation * chore: lint * imp: error msg * test: fix tests * docs: document the validation change --------- Co-authored-by: Alex | Cosmos Labs <alex@cosmoslabs.io> (cherry picked from commit 7574333) * fix: test compilation * fix: remove non-existant field * docs: update changelog --------- Co-authored-by: srdtrk <59252793+srdtrk@users.noreply.github.com> Co-authored-by: Alex | Cosmos Labs <alex@cosmoslabs.io> Co-authored-by: srdtrk <srdtrk@hotmail.com>
Replaces github.com/CosmWasm/wasmvm/v2 with github.com/CosmWasm/wasmvm/v3 (v3.0.2) across all 08-wasm module source files and go.mod.
Signed-off-by: TwiceBurnt <169301814+2xburnt@users.noreply.github.com>
crucible-burnt
left a comment
There was a problem hiding this comment.
✅ Approved — ICS-27 ProtoJSON validation hardening + wasmvm v3 upgrade.
What This Does
Adds a round-trip validation to DeserializeCosmosTx for ProtoJSON encoding: after unmarshalling the CosmosTx, it re-marshals to JSON and compares with the original input via equalJSON(). This catches:
- Missing optional fields (e.g.,
metadataonMsgVote) - Numeric enum values instead of string names (e.g.,
1vs"VOTE_OPTION_YES") - Numeric integer fields instead of string (e.g.,
1vs"1"forproposal_id)
Security Impact
This is a significant hardening for ICA host chains. Without this check, ProtoJSON deserialization silently accepts multiple representations of the same message (numeric vs string enums, missing optional fields with zero defaults). An attacker could craft packets that deserialize correctly but differ from the canonical representation — potentially exploiting discrepancies between chains or relayers that parse the same JSON differently.
This aligns with upstream cosmos/ibc-go#8734 (v10.5.0).
Code Review
codec.go — equalJSON() helper:
- Uses
json.Unmarshalintoany+reflect.DeepEqual— correct approach for semantic JSON comparison (ignores whitespace/formatting, compares structure). ⚠️ Minor:reflect.DeepEqualtreatsfloat64(1)andfloat64(1.0)as equal (both unmarshal tofloat64from JSON), which is the correct behavior here since JSON numbers are all floats.
Test changes (relay_test.go):
- Existing passing tests updated to use strict ProtoJSON format (string enums, string integers, explicit optional fields)
- 3 new failure test cases added:
- Missing optional
metadatafield → rejected ✅ - Integer enum
"option": 1instead of string → rejected ✅ - Integer
"proposal_id": 1instead of string"1"→ rejected ✅
- Missing optional
- Test naming standardized to
success:/failure:prefix — nice consistency improvement
Dependency updates:
wasmvm/v2→wasmvm/v3(go.sum cleanup confirms v2 removed)ibc-go/v10internal refv10.3.0→v10.4.0- e2e deps downgraded (
docker28→27,gogoproto1.7.2→1.7.0,interchaintestv10.0.1→v10.0.0) — presumably aligning with tested versions root.goupdated to checkwasmvm/v3path
One note on the transfer test:
"encoding": ""There's a trailing blank line after this field in the JSON literal (two newlines before }). Cosmetic only — equalJSON handles whitespace correctly.
Verdict
Clean, targeted security hardening. Test coverage is thorough for the new validation. No functional concerns. 🔥
This pull request introduces enhanced validation for ProtoJSON unmarshalling in ICS-27 Interchain Accounts (ICA), updates dependencies, and improves test coverage for stricter JSON handling. The main changes are grouped into protocol improvements, dependency updates, and testing enhancements.
Protocol Improvements
Dependency Updates
github.com/cosmos/ibc-go/v10fromv10.3.0tov10.4.0in bothe2e/go.modandmodules/light-clients/08-wasm/go.mod. [1] [2]github.com/CosmWasm/wasmvmfromv2tov3and updated all relevant import paths throughout the WASM light client modules. [1] [2] [3] [4] [5] [6] [7]Testing Enhancements
Changelog
v10.5.0documenting the enhanced ProtoJSON validation in ICS-27 ICA.