Skip to content

fix(network-driver): bound tcc query request size and item count (#2050) - #2264

Draft
AkramBitar wants to merge 1 commit into
mainfrom
fix-2050-tcc-query-limits
Draft

fix(network-driver): bound tcc query request size and item count (#2050)#2264
AkramBitar wants to merge 1 commit into
mainfrom
fix-2050-tcc-query-limits

Conversation

@AkramBitar

@AkramBitar AkramBitar commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Problem

The chaincode's read-only queries — queryStates, queryTokens, areTokensSpent — perform one
ledger read per element
of a caller-supplied JSON array, with no cap on array length or payload
size. invoke is bounded by driver.ResourceLimits; the query path is not, and it doesn't go
through the size-limited transaction-submission flow either.

Today: attacker sends queryStates an array of 10M keys (no signature, wallet or privileges
needed) → the whole array is unmarshalled → 10M GetState calls in one invocation → the peer is
tied up. Repeat concurrently for resource exhaustion. Same for the other two.

Fix

Two limits, both applied before any work proportional to the input:

Limit Default Checked
MaxQueryRequestBytes 1 MiB before json.Unmarshal
MaxQueryItems 4096 after decode, before the first ledger read

After: oversized payload → rejected before decoding, zero GetState calls. Retry with 5,000
short keys → item cap rejects it before the read loop, zero reads. A legitimate 4,096-key request
is served exactly as before.

WithDefaults() replaces any field left unset or negative, so an unconfigured chaincode is still
bounded and a config typo can't disable a limit. The standalone process overrides via
TOKEN_QUERY_MAX_REQUEST_BYTES / TOKEN_QUERY_MAX_ITEMS. Not consensus-relevant — the query path
writes nothing and isn't an endorsement boundary — so no lockstep rollout needed.

Bonus: a panic the new fuzzing found

["queryTokens", "[null]"] — 8 bytes, inside every limit — panicked: a JSON null decodes to a nil
*token.ID that translator.QueryTokens dereferenced. Invoke's recover() meant no peer crash,
but every such request logged a full stack trace, and direct translator callers have no recovery at
all. Fixed at the root; both triggering payloads are kept in the fuzz corpus.

Tests

  • Exact boundaries for both limits and the env provider.
  • Per query function: over-count and oversize each rejected with zero GetState calls; at exactly
    MaxQueryItems served with one read per element; unconfigured chaincode still bounded. Verified
    these fail with the guards removed.
  • One fuzz target per query function — the surface behind the shared check differs (queryStates uses
    the string as a key verbatim; areTokensSpent adds validator init and, with graph hiding, the
    composite-key builder; queryTokens decodes structs). Nightly matrix entries for all three.
    f.Add carries the seed shapes; testdata/fuzz/ holds only the 8 files that benefit from being on
    disk — the MaxQueryItems boundary pair per target and the two crash reproducers, which report as
    named subtests and outlive the build cache the generated corpus lives in.
  • make checks and make lint-auto-fix clean; ./token/services/network/... green, tcc also under
    -race.

Docs

New docs/security/tcc_query_limits.md, linked from docs/services/network-fabric.md and
docs/configuration.md.

docs/configuration.md is also touched by #1925 (which bounds the storage service and doesn't
touch tcc/) — trivial rebase on that one file for whichever merges second.

Fixes #2050

🤖 Generated with Claude Code

@AkramBitar AkramBitar added this to the Q3/26 milestone Aug 18, 2026
@AkramBitar AkramBitar self-assigned this Aug 18, 2026
@AkramBitar
AkramBitar force-pushed the fix-2050-tcc-query-limits branch from 73a8bac to 6ae8269 Compare August 18, 2026 21:58
The chaincode's read-only queries — queryStates, queryTokens, areTokensSpent —
performed one ledger read per element of an untrusted JSON array, with no cap on
array length or payload size. Unlike invoke, bounded by driver.ResourceLimits,
they are not reached through the size-limited transaction-submission flow, so
any client able to call them could drive an unbounded number of GetState calls
from a single request.

Add tcc.QueryLimits: MaxQueryRequestBytes (1 MiB, checked before the JSON
decode) and MaxQueryItems (4096, checked before the first ledger read). Defaults
replace any field left unset or negative, so an unconfigured chaincode is still
bounded; the standalone chaincode process overrides them via
TOKEN_QUERY_MAX_REQUEST_BYTES / TOKEN_QUERY_MAX_ITEMS. These limits are not
consensus-relevant: the query path performs no writes and is not an endorsement
boundary.

Also fix a nil-pointer dereference the new fuzzing found: a `null` element in a
queryTokens array decodes to a nil *token.ID that translator.QueryTokens
dereferenced.

Tests cover the exact boundaries, rejection with zero GetState calls for each of
the three query functions, and one fuzz target per function (persisted corpora
plus nightly matrix entries).

Fixes #2050

Signed-off-by: AkramBitar <akram@il.ibm.com>
@AkramBitar
AkramBitar force-pushed the fix-2050-tcc-query-limits branch from 6ae8269 to 6f832b7 Compare August 18, 2026 22:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

network-driver: tcc queryStates/queryTokens/areTokensSpent have no request-size limit (resource exhaustion)

1 participant