fix: F-2026-18821 | [Dual Defense] Public InboundKeys Query Runs Unbounded Quadratic Solana Base58 Decode - #329
Merged
Merged
Conversation
…h band Only 64..88 base58 chars can decode to 64 bytes, so gating the decode on that band is output-equivalent. Also cap tx_hash on the unauthenticated InboundKeys query.
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
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.
Finding
Querier.InboundKeysis an unauthenticated gRPC/REST helper that derives the canonical UTX id and inbound ballot id for an inbound. Its entire input validation was a nil check. It reads no state, so it consumes no gas — Cosmos meters store operations, not CPU.It then canonicalizes the tx hash three times:
inbound.Canonicalize(), thenGetInboundBallotKeyandGetInboundUniversalTxKey, both of which self-canonicalize by design.For a
solana:source chain each of those reachedcanonicalizeSolanaTxHash, which calledbase58.Decodeon any base58-alphabet string of any length and — when the result wasn't exactly 64 bytes — discarded it and returned the input unchanged.mr-tron/base58v1.2.0's decoder is quadratic: for each ofncharacters it walksceil(n/4)limbs.So a caller sends a 100,000-character string, the node does the full Θ(n²) decode three times, throws all three results away, and returns the input.
Measured locally (machine under load, so order-of-magnitude):
Reachable on donut today.
uexecutor.v1.Queryroutes through the public CometBFT RPC — verified with a normal-sized benign payload (/abci_query?path="/uexecutor.v1.Query/Params"→code: 0).InboundKeysis an RPC on that same service, so it's reachable with no auth and no fee.The consensus path is in better shape:
msgServer.VoteInboundchecksIsBondedUniversalValidatorandIsTombstonedUniversalValidatorbeforeKeeper.VoteInbound, so only a bonded, non-tombstoned UV can drive the same decode inside DeliverTx. Fixed anyway, since it's the same shared helper.Why this is safe to change on a consensus-path helper
A base58 string can decode to exactly 64 bytes only if its length is in [64, 88] — 88 is
ceil(512 / log2(58)), 64 is the all-zero case where each leading zero byte encodes as one'1'. I verified the band by exhaustive probe rather than deriving it.Anything outside that band already fell through to
return canonunchanged. Gating the decode on the band is therefore output-equivalent for every possible input: same canonical string, same ballot key, same UTX key. No state-machine break, no upgrade handler.The equivalence test asserts exactly this against a reference implementation of the old behaviour.
Changes
utils/canonical.go—canonicalizeSolanaTxHashattemptsbase58.Decodeonly when64 <= len(canon) <= 88. This is the single choke point, so it fixes the querier, the vote path andadmin_revert.gotogether.x/uexecutor/keeper/query_keys.go—maxQueryTxHashLen = 128;InboundKeysrejects an oversizedtx_hashwithInvalidArgument. Defence-in-depth on the unauthenticated surface. Deliberately not applied toraw_payload/verification_data(legitimately long), and not pushed down intoutils.Canonicalize*— the vote path must stay lenient, since a malformed inbound still has to produce a UTX, and changing shared canonicalization would move ballot keys.Tests
utils/canonical_test.go:0x-hexx/uexecutor/keeper/query_keys_test.go: oversizedtx_hashrejected fast; real Solana signature accepted; 128 chars accepted, 129 rejected.Mutation-verified — each half reverted, re-run, restored:
OversizedInputDoesNotDecodefails (3.56s vs the 1s bound); equivalence and real-signature tests still pass, confirming the band changes no outputInboundKeyscapRejectsOversizedTxHashandTxHashAtCapIsAcceptedfailHacken's remediations
GetInboundBallotKey/GetInboundUniversalTxKeyself-canonicalize deliberately ("any caller gets one ballot per logical event"), and that guard is worth keeping. It's also moot: a well-formed hash becomes0x…on the first pass and short-circuits, so the repeat cost only ever hit malformed input, which rec 1 removes.[64, 88]bound rather than a one-sided≤ 128; the band is exact.max_body_bytesis 1 MB, which still admits n ≈ 1e6, far worse than anything measured here. Ops limits shrink the blast radius; only the code cap closes it.Follow-up (not in this PR)
solanaAddressBytes/AddressToBytes32has the identical unbounded-decode shape, expecting 32 bytes → band[32, 44]. It doesn't exist onaudit-fixes— it's on the PC20 branch — so it should get the same one-liner when PC20 lands.