Skip to content

Commit 73a8bac

Browse files
committed
test(network-driver): fuzz areTokensSpent and queryTokens query limits
FuzzQueryStatesLedgerReadsAreBounded covered only queryStates. The other two query functions share the same attack surface — an untrusted JSON array whose length decides how many ledger reads one invocation performs — but not the code behind it, so the existing target exercised none of it: - queryStates uses each decoded string as a ledger key verbatim. - areTokensSpent initializes the validator from the public parameters first and, when graph hiding is on, pushes every caller-supplied id through the key translator's composite-key builder — UTF-8 validation and rune scanning of attacker-controlled text. - queryTokens decodes an array of token.ID structs (string plus uint64) rather than plain strings, and derives an output key from each element. Add one target per function, each asserting the same invariant: for arbitrary caller bytes the function never panics and never performs more than MaxQueryItems ledger reads. Separate targets are also required by Go fuzzing, which needs one target per input shape; areTokensSpent additionally fuzzes the graph-hiding branch as a bool argument, so both branches are covered. Each target gets a persisted seed corpus covering both limit boundaries plus empty, truncated, wrong-shape and rejected-rune payloads, and a nightly matrix entry. The queryTokens corpus keeps the two payloads that exposed the nil *token.ID dereference fixed in the previous commit, so the panic is now caught by a plain `go test` run and not only under -fuzz. Deduplicate the public-parameters test fixture: the existing writePublicParamsFile now delegates to a testing.TB helper the fuzz targets can use via f.Setenv. Signed-off-by: AkramBitar <akram@il.ibm.com>
1 parent 5191286 commit 73a8bac

30 files changed

Lines changed: 205 additions & 38 deletions

.github/workflows/nightly-fuzz.yml

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,9 +123,15 @@ jobs:
123123
- name: driver-config-key-field-round-trip
124124
pkg: ./token/driver
125125
func: FuzzConfigKeyFieldRoundTrip
126-
- name: tcc-query-limits
126+
- name: tcc-query-states-limits
127127
pkg: ./token/services/network/fabric/tcc
128128
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
129135

130136
steps:
131137
- name: Checkout code

docs/security/tcc_query_limits.md

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,22 @@ the larger batch no longer fits.
8686
unconfigured `TokenChaincode` is shown to still be bounded by the defaults.
8787
- **Provider tests**: unset environment (resolves to defaults), partial override (unset fields still
8888
default), and an unparseable value (returns an error).
89-
- **Fuzzing**: `FuzzQueryStatesLedgerReadsAreBounded` asserts that for arbitrary caller bytes
90-
`queryStates` never panics and never performs more than `MaxQueryItems` ledger reads. Its seed
91-
corpus (`testdata/fuzz/FuzzQueryStatesLedgerReadsAreBounded/`) covers both boundaries plus empty,
92-
truncated and wrong-shape payloads, and it runs nightly via
93-
[`.github/workflows/nightly-fuzz.yml`](../../.github/workflows/nightly-fuzz.yml).
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+
Each has a persisted seed corpus under `testdata/fuzz/<TargetName>/` covering both limit
100+
boundaries plus empty, truncated, wrong-shape and rejected-rune payloads, and all three run
101+
nightly via [`.github/workflows/nightly-fuzz.yml`](../../.github/workflows/nightly-fuzz.yml).
102+
103+
`FuzzQueryTokensLedgerReadsAreBounded` found a pre-existing nil-pointer dereference on its first
104+
run: a `null` element in the JSON array decodes to a nil `*token.ID`, which
105+
`translator.QueryTokens` dereferenced. It now reports a nil entry as an invalid request instead;
106+
the payload that triggered it is kept in the corpus
107+
(`nil-elements-panic-regression`, `single-nil-element-panic-regression`).

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

Lines changed: 112 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ SPDX-License-Identifier: Apache-2.0
77
package tcc_test
88

99
import (
10+
"bytes"
1011
"encoding/base64"
1112
"encoding/json"
1213
"os"
@@ -21,20 +22,29 @@ import (
2122
"github.com/stretchr/testify/require"
2223
)
2324

24-
// newTestChaincode returns a TokenChaincode with the given query limits, a fake ledger stub, and
25-
// the public parameters wired through a temporary file (the mechanism the real chaincode uses when
26-
// the parameters are not burnt into the binary).
27-
func newTestChaincode(t *testing.T, limits tcc.QueryLimits) (*tcc.TokenChaincode, *mock.ChaincodeStubInterface) {
28-
t.Helper()
25+
// publicParamsFile writes public parameters to a temporary file and returns its path. The chaincode
26+
// resolves its parameters from PublicParamsPathVarEnv when they are not burnt into the binary, and
27+
// AreTokensSpent needs them: it initializes the validator before serving a request.
28+
func publicParamsFile(tb testing.TB) string {
29+
tb.Helper()
30+
31+
ppFile := filepath.Join(tb.TempDir(), "pp")
32+
require.NoError(tb, os.WriteFile(ppFile, []byte(base64.StdEncoding.EncodeToString([]byte("public parameters"))), 0o600))
2933

30-
ppFile := filepath.Join(t.TempDir(), "pp")
31-
require.NoError(t, os.WriteFile(ppFile, []byte(base64.StdEncoding.EncodeToString([]byte("public parameters"))), 0o600))
32-
t.Setenv(tcc.PublicParamsPathVarEnv, ppFile)
34+
return ppFile
35+
}
36+
37+
// newChaincode returns a TokenChaincode with the given query limits and a fake ledger stub that
38+
// answers every read. graphHiding selects which branch of AreTokensSpent runs: with it on, every
39+
// caller-supplied id is pushed through the key translator before the read.
40+
func newChaincode(limits tcc.QueryLimits, graphHiding bool) (*tcc.TokenChaincode, *mock.ChaincodeStubInterface) {
41+
ppm := &mock.PublicParametersManager{}
42+
ppm.GraphHidingReturns(graphHiding)
3343

3444
cc := &tcc.TokenChaincode{
3545
QueryLimits: limits,
3646
TokenServicesFactory: func([]byte) (tcc.PublicParameters, tcc.Validator, error) {
37-
return &mock.PublicParametersManager{}, &mock.Validator{}, nil
47+
return ppm, &mock.Validator{}, nil
3848
},
3949
}
4050
stub := &mock.ChaincodeStubInterface{}
@@ -44,6 +54,15 @@ func newTestChaincode(t *testing.T, limits tcc.QueryLimits) (*tcc.TokenChaincode
4454
return cc, stub
4555
}
4656

57+
// newTestChaincode returns a TokenChaincode with the given query limits, a fake ledger stub, and
58+
// the public parameters wired through a temporary file.
59+
func newTestChaincode(t *testing.T, limits tcc.QueryLimits) (*tcc.TokenChaincode, *mock.ChaincodeStubInterface) {
60+
t.Helper()
61+
t.Setenv(tcc.PublicParamsPathVarEnv, publicParamsFile(t))
62+
63+
return newChaincode(limits, false)
64+
}
65+
4766
// stateKeys returns n state keys of a realistic shape and size.
4867
func stateKeys(n int) []string {
4968
keys := make([]string, n)
@@ -182,28 +201,96 @@ func TestQueryFunctions_UnconfiguredChaincodeStillBounded(t *testing.T) {
182201
}
183202
}
184203

204+
// fuzzLimits are the limits the fuzz targets enforce. They are far tighter than the defaults so a
205+
// mutator working on short inputs still crosses both boundaries regularly.
206+
var fuzzLimits = tcc.QueryLimits{MaxQueryRequestBytes: 4096, MaxQueryItems: 8}
207+
208+
// stringArraySeeds returns payload seeds for the query functions that take a JSON array of strings.
209+
func stringArraySeeds() [][]byte {
210+
oversized := append([]byte(`["`), append(bytes.Repeat([]byte("a"), 8192), []byte(`"]`)...)...)
211+
212+
return [][]byte{
213+
[]byte(`["a","b"]`), // valid, under the item cap
214+
[]byte(`[]`), // empty array
215+
[]byte(``), // empty payload
216+
[]byte(`["a","b`), // truncated
217+
[]byte(`{"a":1}`), // wrong JSON shape
218+
[]byte(`["a","b","c","d","e","f","g","h"]`), // exactly at the item cap
219+
[]byte(`["a","b","c","d","e","f","g","h","i"]`), // one over the item cap
220+
[]byte("[\"\u0000\",\"\U0010ffff\"]"), // runes the composite-key builder rejects
221+
[]byte(`["\udc00"]`), // invalid UTF-8 after JSON unescaping
222+
oversized, // over the byte cap
223+
}
224+
}
225+
185226
// FuzzQueryStatesLedgerReadsAreBounded asserts the invariant the limits exist to guarantee: whatever
186227
// bytes an untrusted caller supplies, QueryStates never panics and never performs more ledger reads
187-
// than MaxQueryItems.
228+
// than MaxQueryItems. QueryStates uses each decoded string as a ledger key verbatim.
188229
func FuzzQueryStatesLedgerReadsAreBounded(f *testing.F) {
189-
limits := tcc.QueryLimits{MaxQueryRequestBytes: 4096, MaxQueryItems: 8}
230+
for _, seed := range stringArraySeeds() {
231+
f.Add(seed)
232+
}
233+
234+
f.Fuzz(func(t *testing.T, raw []byte) {
235+
cc, stub := newChaincode(fuzzLimits, false)
236+
237+
require.NotNil(t, cc.QueryStates(raw, stub))
238+
require.LessOrEqual(t, stub.GetStateCallCount(), fuzzLimits.MaxQueryItems)
239+
})
240+
}
241+
242+
// FuzzAreTokensSpentLedgerReadsAreBounded fuzzes the same invariant for AreTokensSpent, which shares
243+
// the array-of-strings attack surface but not the code behind it: it initializes the validator from
244+
// the public parameters first, and — when graph hiding is on — pushes every caller-supplied id
245+
// through the key translator's composite-key builder (UTF-8 validation and rune scanning of
246+
// attacker-controlled text) before the read. The graphHiding argument fuzzes both branches.
247+
func FuzzAreTokensSpentLedgerReadsAreBounded(f *testing.F) {
248+
f.Setenv(tcc.PublicParamsPathVarEnv, publicParamsFile(f))
249+
250+
for _, seed := range stringArraySeeds() {
251+
f.Add(seed, false)
252+
f.Add(seed, true)
253+
}
254+
255+
f.Fuzz(func(t *testing.T, raw []byte, graphHiding bool) {
256+
cc, stub := newChaincode(fuzzLimits, graphHiding)
257+
258+
require.NotNil(t, cc.AreTokensSpent(raw, stub))
259+
require.LessOrEqual(t, stub.GetStateCallCount(), fuzzLimits.MaxQueryItems)
260+
})
261+
}
262+
263+
// FuzzQueryTokensLedgerReadsAreBounded fuzzes the same invariant for QueryTokens, whose surface is
264+
// wider again: it decodes an array of token.ID structs (a string plus a uint64) rather than plain
265+
// strings, and derives an output key from each one, so both the decoder and the key builder see
266+
// attacker-controlled input.
267+
func FuzzQueryTokensLedgerReadsAreBounded(f *testing.F) {
268+
ids := func(n int) []byte {
269+
raw, err := json.Marshal(tokenIDs(n))
270+
if err != nil {
271+
f.Fatal(err)
272+
}
273+
274+
return raw
275+
}
190276

191-
f.Add([]byte(`["a","b"]`)) // valid, under the item cap
192-
f.Add([]byte(`[]`)) // empty array
193-
f.Add([]byte(``)) // empty payload
194-
f.Add([]byte(`["a","b`)) // truncated
195-
f.Add([]byte(`{"a":1}`)) // wrong JSON shape
196-
f.Add([]byte(`["a","b","c","d","e","f","g","h"]`)) // exactly at the item cap
197-
f.Add([]byte(`["a","b","c","d","e","f","g","h","i"]`)) // one over the item cap
198-
f.Add(append([]byte(`["`), append(make([]byte, 8192), []byte(`"]`)...)...)) // over the byte cap
277+
f.Add(ids(2)) // valid, under the item cap
278+
f.Add(ids(fuzzLimits.MaxQueryItems)) // exactly at the item cap
279+
f.Add(ids(fuzzLimits.MaxQueryItems + 1)) // one over the item cap
280+
f.Add([]byte(`[]`)) // empty array
281+
f.Add([]byte(``)) // empty payload
282+
f.Add([]byte(`[{"tx_id":"a","index":1`)) // truncated
283+
f.Add([]byte(`["a"]`)) // wrong element type
284+
f.Add([]byte(`[null,null]`)) // nil elements
285+
f.Add([]byte(`[{"tx_id":"a","index":-1}]`)) // index out of range for uint64
286+
f.Add([]byte("[{\"tx_id\":\"\u0000\"}]")) // a rune the composite-key builder rejects
287+
f.Add([]byte(`[{"tx_id":"a","index":18446744073709551615}]`)) // max uint64 index
288+
f.Add(append([]byte(`[{"tx_id":"`), append(bytes.Repeat([]byte("a"), 8192), []byte(`"}]`)...)...)) // over the byte cap
199289

200290
f.Fuzz(func(t *testing.T, raw []byte) {
201-
cc := &tcc.TokenChaincode{QueryLimits: limits}
202-
stub := &mock.ChaincodeStubInterface{}
203-
stub.GetTxIDReturns("txid")
291+
cc, stub := newChaincode(fuzzLimits, false)
204292

205-
resp := cc.QueryStates(raw, stub)
206-
require.NotNil(t, resp)
207-
require.LessOrEqual(t, stub.GetStateCallCount(), limits.MaxQueryItems)
293+
require.NotNil(t, cc.QueryTokens(raw, stub))
294+
require.LessOrEqual(t, stub.GetStateCallCount(), fuzzLimits.MaxQueryItems)
208295
})
209296
}

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

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,6 @@ SPDX-License-Identifier: Apache-2.0
77
package tcc_test
88

99
import (
10-
"encoding/base64"
11-
"os"
12-
"path/filepath"
1310
"sync"
1411
"sync/atomic"
1512
"testing"
@@ -25,10 +22,7 @@ import (
2522
// parameters, so that TokenChaincode.Params succeeds and initialization reaches the factory.
2623
func writePublicParamsFile(t *testing.T) {
2724
t.Helper()
28-
29-
path := filepath.Join(t.TempDir(), "pp")
30-
require.NoError(t, os.WriteFile(path, []byte(base64.StdEncoding.EncodeToString([]byte("public parameters"))), 0o600))
31-
t.Setenv(tcc.PublicParamsPathVarEnv, path)
25+
t.Setenv(tcc.PublicParamsPathVarEnv, publicParamsFile(t))
3226
}
3327

3428
// TestGetValidatorRetriesAfterFailedInitialization checks that a failed initialization attempt is
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
go test fuzz v1
2+
[]byte("[\"\\u0000\",\"\\U0010ffff\"]")
3+
bool(true)
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
go test fuzz v1
2+
[]byte("[]")
3+
bool(false)
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
go test fuzz v1
2+
[]byte("[]")
3+
bool(true)
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
go test fuzz v1
2+
[]byte("[\"\\udc00\"]")
3+
bool(true)
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
go test fuzz v1
2+
[]byte("[\"k0\",\"k1\",\"k2\",\"k3\",\"k4\",\"k5\",\"k6\",\"k7\"]")
3+
bool(false)
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
go test fuzz v1
2+
[]byte("[\"k0\",\"k1\",\"k2\",\"k3\",\"k4\",\"k5\",\"k6\",\"k7\"]")
3+
bool(true)

0 commit comments

Comments
 (0)