Skip to content

Commit 6f832b7

Browse files
committed
fix(network-driver): bound tcc query request size and item count
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>
1 parent 5c8aafc commit 6f832b7

20 files changed

Lines changed: 906 additions & 8 deletions

File tree

.github/workflows/nightly-fuzz.yml

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,15 @@ jobs:
123123
- name: driver-config-key-field-round-trip
124124
pkg: ./token/driver
125125
func: FuzzConfigKeyFieldRoundTrip
126+
- name: tcc-query-states-limits
127+
pkg: ./token/services/network/fabric/tcc
128+
func: FuzzQueryStatesLedgerReadsAreBounded
129+
- name: tcc-are-tokens-spent-limits
130+
pkg: ./token/services/network/fabric/tcc
131+
func: FuzzAreTokensSpentLedgerReadsAreBounded
132+
- name: tcc-query-tokens-limits
133+
pkg: ./token/services/network/fabric/tcc
134+
func: FuzzQueryTokensLedgerReadsAreBounded
126135

127136
steps:
128137
- name: Checkout code

docs/configuration.md

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -551,6 +551,25 @@ out as a coordinated configuration change before any peer relies on the new valu
551551

552552
---
553553

554+
### Optional: token chaincode query limits (environment only)
555+
556+
The read-only query functions of the token chaincode (`queryStates`, `queryTokens`,
557+
`areTokensSpent`) perform one ledger read per element of the caller-supplied array, so they are
558+
bounded independently of `token.validation.limits`. These limits live in the chaincode process, not
559+
in the FSC node's configuration file, and are read from the environment:
560+
561+
| Environment variable | Default | Bounds |
562+
| --- | --- | --- |
563+
| `TOKEN_QUERY_MAX_REQUEST_BYTES` | 1048576 (1 MiB) | Raw size of the query argument, checked before it is decoded |
564+
| `TOKEN_QUERY_MAX_ITEMS` | 4096 | Number of elements, checked before the first ledger read |
565+
566+
Both are optional; an unset variable resolves to its default, and an unparseable value is a startup
567+
error. Unlike `token.validation.limits` these values are **not** consensus-relevant — the query path
568+
performs no writes and is not an endorsement boundary — so they do not need to match across peers.
569+
See [Token Chaincode Query Limits](security/tcc_query_limits.md).
570+
571+
---
572+
554573
### Optional: token.fabricx.lookup
555574

556575
If not specified, the default configuration is:

docs/security/tcc_query_limits.md

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
# Token Chaincode Query Limits
2+
3+
This page describes the limits enforced on the token chaincode's read-only query functions, why
4+
they exist, and how to configure them.
5+
6+
## Why limits exist
7+
8+
The token chaincode (`token/services/network/fabric/tcc`) exposes three read-only query functions:
9+
10+
| Function | Argument | Work per element |
11+
| --- | --- | --- |
12+
| `queryStates` | JSON array of state keys | one `GetState` |
13+
| `queryTokens` | JSON array of `token.ID`s | one `GetState` |
14+
| `areTokensSpent` | JSON array of token/serial-number keys | one `GetState` |
15+
16+
Every element the caller supplies translates 1:1 into a ledger read inside a *single* chaincode
17+
invocation. The `invoke` path is bounded by `driver.ResourceLimits` (see
18+
[Validator Resource Limits](../drivers/validation-resource-limits.md)), which rejects an oversized
19+
token request before any per-element work. The query path had no analogous guard: a single request
20+
carrying an arbitrarily long array drove an unbounded number of ledger reads, and — unlike `invoke`
21+
— it does not go through the size-limited transaction-submission flow. That is a resource-exhaustion
22+
/ peer-slowdown vector reachable by any client that can call the chaincode's query path, with no
23+
elevated privileges (issue #2050).
24+
25+
## Configuration mechanism
26+
27+
Limits are held in `tcc.QueryLimits` (`token/services/network/fabric/tcc/querylimits.go`) and read
28+
from the `QueryLimits` field of `tcc.TokenChaincode`:
29+
30+
| Field | Default | Checked | Enforced |
31+
| --- | --- | --- | --- |
32+
| `MaxQueryRequestBytes` | 1 MiB | Raw size of the query argument | Before `json.Unmarshal`, so an oversized payload never reaches an allocation proportional to its own size |
33+
| `MaxQueryItems` | 4096 | Number of elements in the decoded array | After the decode and **before the first ledger read** |
34+
35+
`MaxQueryItems` is the meaningful bound — it caps how many ledger reads one invocation can perform.
36+
`MaxQueryRequestBytes` is deliberately high enough that a full 4096-element batch of realistic state
37+
keys still fits, so the two limits do not shadow each other and the binding limit is predictable; it
38+
exists to reject a payload before decoding, including one made of few but enormous elements. Both
39+
defaults are far above the batch sizes produced by any in-tree caller
40+
(`lookup.DeliveryScanQueryByID`, `tokenFetcher.QueryTokens`, `spentTokenFetcher.QuerySpentTokens`).
41+
42+
Violations return a typed error — `tcc.ErrQueryRequestTooLarge` or `tcc.ErrTooManyQueryItems`
43+
wrapping the effective limit, surfaced to the caller as a chaincode error response.
44+
45+
`QueryLimits.WithDefaults()` overlays `DefaultQueryLimits()` onto any field that is not a positive
46+
value, and the chaincode applies it on every query. Consequently a `TokenChaincode` built without
47+
setting `QueryLimits` is still bounded by the defaults, and a partially-specified override (or a
48+
negative value from a configuration typo) can never silently disable a limit.
49+
50+
The standalone chaincode process (`token/services/network/fabric/tcc/main/main.go`) has no
51+
configuration service wired, so it resolves the limits from the environment via
52+
`tcc.EnvQueryLimitsProvider` — mirroring `tcc.EnvResourceLimitsProvider` for validation limits:
53+
54+
| Environment variable | Field |
55+
| --- | --- |
56+
| `TOKEN_QUERY_MAX_REQUEST_BYTES` | `MaxQueryRequestBytes` |
57+
| `TOKEN_QUERY_MAX_ITEMS` | `MaxQueryItems` |
58+
59+
Each variable is optional; an unset variable leaves the field at zero, which `WithDefaults` then
60+
replaces with the default. An unparseable value is a startup error, not a silent fallback.
61+
62+
## Relationship to the consensus-safety contract
63+
64+
Unlike `driver.ResourceLimits`, these limits are **not** consensus-relevant. The query functions
65+
perform no writes and are reached through the query/evaluate path rather than through endorsement,
66+
so a peer configured with a stricter value only refuses a request another peer would serve — it
67+
cannot make two peers disagree on a transaction's validity. Query limits are therefore an
68+
operational knob per chaincode process, not a value that must be rolled out in lockstep.
69+
70+
## Choosing and changing these values
71+
72+
Clients that legitimately need to look up more than `MaxQueryItems` keys should chunk their
73+
requests. If a deployment instead raises `TOKEN_QUERY_MAX_ITEMS`, keep in mind that the value
74+
directly bounds the ledger reads a single unauthenticated request can trigger — raise it only as far
75+
as observed legitimate batch sizes require, and raise `TOKEN_QUERY_MAX_REQUEST_BYTES` alongside it if
76+
the larger batch no longer fits.
77+
78+
## Testing
79+
80+
- **Exact-boundary unit tests** (`querylimits_test.go`): every field asserts `limit-1` / `limit`
81+
succeed and `limit+1` fails with the specific typed error, against both the defaults and an
82+
injected override.
83+
- **Regression tests** (`queryguard_test.go`): for each of the three query functions, an
84+
over-counted request and an oversized payload are both rejected with **zero** `GetState` calls; a
85+
request at exactly `MaxQueryItems` is served and performs exactly one read per element; and an
86+
unconfigured `TokenChaincode` is shown to still be bounded by the defaults.
87+
- **Provider tests**: unset environment (resolves to defaults), partial override (unset fields still
88+
default), and an unparseable value (returns an error).
89+
- **Fuzzing**: one target per query function, each asserting that for arbitrary caller bytes the
90+
function never panics and never performs more than `MaxQueryItems` ledger reads. The three are
91+
separate targets because the surface behind the shared limit check differs:
92+
93+
| Target | Covers |
94+
| --- | --- |
95+
| `FuzzQueryStatesLedgerReadsAreBounded` | array-of-strings decode; each string is used as a ledger key verbatim |
96+
| `FuzzAreTokensSpentLedgerReadsAreBounded` | same decode, plus validator initialization from the public parameters and — with graph hiding on, fuzzed as a second argument — the key translator's composite-key builder (UTF-8 validation and rune scanning of attacker-controlled text) |
97+
| `FuzzQueryTokensLedgerReadsAreBounded` | array-of-`token.ID` decode (string + `uint64`) and output-key derivation from each element |
98+
99+
In-code `f.Add` seeds cover both limit boundaries plus empty, truncated, wrong-shape and
100+
rejected-rune payloads. `testdata/fuzz/<TargetName>/` holds only what benefits from living on disk
101+
— the `MaxQueryItems` boundary pair (the limit that bounds ledger reads) and the crash
102+
reproducers, which Go writes there itself and whose named files report as named subtests. The
103+
corpus fuzzing *generates* is not committed: it lives in `$GOCACHE/fuzz` and CI restores it only
104+
best-effort, so these files are the durable floor. All three targets run nightly via
105+
[`.github/workflows/nightly-fuzz.yml`](../../.github/workflows/nightly-fuzz.yml).
106+
107+
`FuzzQueryTokensLedgerReadsAreBounded` found a pre-existing nil-pointer dereference on its first
108+
run: a `null` element in the JSON array decodes to a nil `*token.ID`, which
109+
`translator.QueryTokens` dereferenced. It now reports a nil entry as an invalid request instead;
110+
the two payloads that triggered it are kept on disk (`nil-elements-panic-regression`,
111+
`single-nil-element-panic-regression`), so the panic is caught by a plain `go test` run.

docs/services/network-fabric.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,16 @@ The chaincode exposes the following functions:
4848
| `areTokensSpent` | Check if tokens are spent | Token IDs, metadata | Boolean array |
4949
| `queryStates` | Query arbitrary state keys | State keys | State values |
5050

51+
### Query Limits
52+
53+
`queryTokens`, `areTokensSpent` and `queryStates` each turn every element of the caller-supplied
54+
array into one ledger read within a single invocation, so both the raw argument size and the number
55+
of elements are bounded before any read happens (`MaxQueryRequestBytes`, default 1 MiB;
56+
`MaxQueryItems`, default 4096). The defaults apply even when the chaincode is built without
57+
configuring them, and the standalone chaincode process can override them via
58+
`TOKEN_QUERY_MAX_REQUEST_BYTES` / `TOKEN_QUERY_MAX_ITEMS`. Clients that need more keys than the cap
59+
must chunk their requests. See [Token Chaincode Query Limits](../security/tcc_query_limits.md).
60+
5161
### Chaincode Deployment
5262

5363
The Token Chaincode must be deployed to the Fabric network before Panurus can operate:
Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
/*
2+
Copyright IBM Corp. All Rights Reserved.
3+
4+
SPDX-License-Identifier: Apache-2.0
5+
*/
6+
7+
package translator_test
8+
9+
import (
10+
"context"
11+
"testing"
12+
13+
"github.com/LFDT-Panurus/panurus/token/services/network/common/rws/keys"
14+
"github.com/LFDT-Panurus/panurus/token/services/network/common/rws/translator"
15+
"github.com/LFDT-Panurus/panurus/token/services/network/common/rws/translator/mock"
16+
"github.com/LFDT-Panurus/panurus/token/token"
17+
"github.com/stretchr/testify/assert"
18+
"github.com/stretchr/testify/require"
19+
)
20+
21+
func newQueryTokensTranslator() (*translator.Translator, *mock.RWSet) {
22+
rws := &mock.RWSet{}
23+
rws.GetStateReturns([]byte("value"), nil)
24+
25+
return translator.New("0", translator.NewRWSetWrapper(rws, tokenNameSpace, "0"), &keys.Translator{}), rws
26+
}
27+
28+
// The ids reach QueryTokens straight from a JSON array decoded on the chaincode's query path, where
29+
// a `null` element decodes to a nil *token.ID. A nil entry must be reported as an invalid request
30+
// rather than dereferenced, and must not stop the ledger read of the valid ids around it.
31+
func TestQueryTokens_NilIDIsRejectedNotDereferenced(t *testing.T) {
32+
for _, tc := range []struct {
33+
name string
34+
ids []*token.ID
35+
}{
36+
{"only a nil id", []*token.ID{nil}},
37+
{"nil id first", []*token.ID{nil, {TxId: "tx", Index: 0}}},
38+
{"nil id last", []*token.ID{{TxId: "tx", Index: 0}, nil}},
39+
{"all nil ids", []*token.ID{nil, nil}},
40+
} {
41+
t.Run(tc.name, func(t *testing.T) {
42+
w, _ := newQueryTokensTranslator()
43+
44+
var res [][]byte
45+
var err error
46+
require.NotPanics(t, func() { res, err = w.QueryTokens(context.Background(), tc.ids) })
47+
require.Error(t, err)
48+
require.ErrorContains(t, err, "nil token id at index")
49+
assert.Nil(t, res)
50+
})
51+
}
52+
}
53+
54+
func TestQueryTokens_ValidIDsAreRead(t *testing.T) {
55+
w, rws := newQueryTokensTranslator()
56+
57+
res, err := w.QueryTokens(context.Background(), []*token.ID{{TxId: "tx", Index: 0}, {TxId: "tx", Index: 1}})
58+
require.NoError(t, err)
59+
assert.Len(t, res, 2)
60+
assert.Equal(t, 2, rws.GetStateCallCount())
61+
}

token/services/network/common/rws/translator/translator.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,15 @@ func (t *Translator) AddPublicParamsDependency() error {
134134
func (t *Translator) QueryTokens(ctx context.Context, ids []*token.ID) ([][]byte, error) {
135135
var res [][]byte
136136
var errs []error
137-
for _, id := range ids {
137+
for i, id := range ids {
138+
if id == nil {
139+
// The ids come from an untrusted caller (the chaincode decodes them straight from a
140+
// JSON array, where a `null` element decodes to a nil pointer), so a nil entry is an
141+
// invalid request, not a programming error to panic on.
142+
errs = append(errs, errors.Errorf("nil token id at index [%d]", i))
143+
144+
continue
145+
}
138146
outputID, err := t.KeyTranslator.CreateOutputKey(id.TxId, id.Index)
139147
if err != nil {
140148
errs = append(errs, errors.Errorf("error creating output ID: %s", err))

token/services/network/fabric/tcc/main/main.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,9 @@ func main() {
6161
limits, err := tcc.NewEnvResourceLimitsProvider().ResourceLimits()
6262
assertNoError(err, "cannot resolve validation resource limits")
6363

64+
queryLimits, err := tcc.NewEnvQueryLimitsProvider().QueryLimits()
65+
assertNoError(err, "cannot resolve query limits")
66+
6467
is := core.NewValidatorDriverService(
6568
limits,
6669
fabtoken.NewValidatorDriver(),
@@ -73,6 +76,7 @@ func main() {
7376
}
7477
err := shim.Start(
7578
&tcc.TokenChaincode{
79+
QueryLimits: queryLimits,
7680
TokenServicesFactory: func(bytes []byte) (tcc.PublicParameters, tcc.Validator, error) {
7781
ppm, err := is.PublicParametersFromBytes(bytes)
7882
if err != nil {
@@ -118,6 +122,7 @@ func main() {
118122
CCID: config.CCID,
119123
Address: config.CCaddress,
120124
CC: &tcc.TokenChaincode{
125+
QueryLimits: queryLimits,
121126
TokenServicesFactory: func(bytes []byte) (tcc.PublicParameters, tcc.Validator, error) {
122127
ppm, err := is.PublicParametersFromBytes(bytes)
123128
if err != nil {

0 commit comments

Comments
 (0)