fix(x/bank): cap denom-unit + alias length during metadata validation#26426
Open
ozpool wants to merge 1 commit into
Open
fix(x/bank): cap denom-unit + alias length during metadata validation#26426ozpool wants to merge 1 commit into
ozpool wants to merge 1 commit into
Conversation
…ion (cosmos#26012) cosmos#26012 flagged that Metadata.Validate has no semantic size limit on DenomUnits or per-unit Aliases, so a malicious actor can submit a metadata payload that bloats SetDenomMetadata storage without paying proportional gas. Add a count cap on each: - MaxDenomUnits = 100 — well above the 2-4 entries real-world tokens publish (uatom/matom/atom-style ladders), generous enough to absorb unusual reference-currency mappings. - MaxDenomUnitAliases = 32 — production usage is one to three; 32 still blocks unbounded payloads. Both caps fire during Metadata.Validate via a single len() check, so the existing genesis / msg_server / proposal validation paths inherit them for free. Tests added in x/bank/types/metadata_test.go cover the new cases: - too many denom units (MaxDenomUnits+1) -> rejected - denom units at exact cap -> accepted - too many aliases on a denom unit (MaxDenomUnitAliases+1) -> rejected x/bank/types tests: ok 0.540s. gofmt + go vet clean.
Contributor
|
PR author is not in the allowed authors list. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Closes #26012.
Metadata.Validatehad no semantic size limit onDenomUnitsor per-unitAliases, so a payload like{ DenomUnits: <1M entries> }passes today's per-entry checks. The issue calls out exactly this: "It makes it easy for malicious actor to construct large message."Add a count cap on each slice. The caps fire during the existing
Validatepath, so genesis ingestion,MsgSetDenomMetadata, and gov-proposal flows all inherit them for free without changing surface or invariants.MaxDenomUnitsuatom/matom/atom). 100 absorbs unusual reference-currency mappings (decimal divisions, alt locale shorthand) without admitting unbounded payloads.MaxDenomUnitAliasesTests
x/bank/types/metadata_test.go::TestMetadataValidatepicks up three new cases:too many denom units (#26012)—MaxDenomUnits + 1entries → rejected.denom units at exact cap— exactlyMaxDenomUnitsentries (with sort + display invariants intact viabuildMetadataWithDenomUnits) → accepted.too many aliases on a denom unit (#26012)—MaxDenomUnitAliases + 1aliases on one unit → rejected.CHANGELOG
Per maintainer convention, leaving the
### Bug Fixesentry for the merger to add at landing time alongside the assigned PR number. Suggested line: