test: add fuzz harness for crypto/action/state codec + primitives#4835
Open
envestcc wants to merge 2 commits into
Open
test: add fuzz harness for crypto/action/state codec + primitives#4835envestcc wants to merge 2 commits into
envestcc wants to merge 2 commits into
Conversation
NewMerkleTree(leaves) for size==1 sets mk.root = leaves[0]. When the single leaf happens to be hash.ZeroHash256, HashTree's sentinel check "if mk.root != ZeroHash256" misreads the state as "root not yet computed" and falls through to the multi-leaf path. There, length becomes mk.size >> 1 == 0, the inner merkle slice is empty, and the final merkle[0] access panics with index out of range. Short-circuit the single-leaf case at the top of HashTree: the root of a 1-leaf tree is the leaf itself, regardless of its value. Found by FuzzMerkleHashTree (added in the following commit). Refs iotexproject#4832
Introduces seven Go native fuzz targets covering the highest-value serialization and cryptographic primitive paths: crypto/ — FuzzMerkleHashTree, FuzzSort, FuzzSortCandidates action/ — FuzzReceiptDeserialize, FuzzLogDeserialize state/ — FuzzAccountDeserialize, FuzzCandidateListDeserialize Each target seeds the corpus with valid round-trip-able instances drawn from known-good shapes, then asserts via deferred recover that the deserializer/primitive never panics on arbitrary input. Findings beyond the seed corpus stay in testdata/fuzz/ on the runner; they are not committed back to the repo. CI integration: a new "Run Fuzz (short)" step in ci.yaml invokes `make fuzz-short` (each target for FUZZTIME=30s, overridable). It is marked continue-on-error while existing panic sites surfaced by the harness are triaged in follow-ups; promote to required once those are resolved. Local 15s/target runs surface two known Account.FromProto panic sites (invalid balance string, unknown AccountType enum) that require a signature change to fix and are intentionally out of scope here. Refs iotexproject#4832
|
CoderZhi
approved these changes
May 25, 2026
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
2 participants
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.
Summary
Introduces seven Go native fuzz targets for the highest-value serialization and cryptographic primitive paths in
crypto/,action/, andstate/, plus amake fuzz-shortMakefile target and an advisory CI step.Also includes a small bug fix to
crypto.Merkle.HashTreethat was surfaced by the new harness during local validation — see commits.This implements item P0.3 from the tracking issue (#4832).
What's added
Each fuzz target seeds its corpus with valid round-trip-able instances and asserts (via
defer recover) that the deserializer/primitive never panics on arbitrary input. Findings beyond the seed corpus stay intestdata/fuzz/on the runner — they are not committed back.Bug surfaced and fixed in this PR
crypto.Merkle.HashTree()panicked withindex out of rangewhen constructed from a single leaf whose hash ishash.ZeroHash256. Root cause:NewMerkleTreesetsmk.root = leaves[0], andHashTree's sentinelif mk.root != ZeroHash256then misreads the state as "root not yet computed" and falls through to the multi-leaf path. Fix: short-circuit the single-leaf case (the root of a 1-leaf tree is the leaf itself).CI integration
A new step "Run Fuzz (short)" invokes
make fuzz-short(each target for 30s by default). It is currentlycontinue-on-error: trueso the harness can land while existing panic sites are triaged. Promote to required once the follow-ups below are resolved.Follow-ups surfaced by the harness
A 15s/target local run found two panic sites in
state.Account.FromProtothat require a signature change (return error rather than panic) to fix and are intentionally out of scope here:Account.FromProtopanics on a non-decimalBalancestring (new(big.Int).SetString(s, 10)returns!ok). Fuzz reproducer: any 32+ byte input where the balance field decodes to a non-decimal string.Account.FromProtopanics on an unknownAccountTypeenum (defensivepanic("unknown account type")). Fuzz reproducer: any input where the type field decodes to an out-of-schema enum value.Both should become
errorreturns. I'll file these as separate issues after this PR lands.Local validation
go test ./crypto/... ./action/... ./state/...— 289 existing tests pass, 37 fuzz seed tests pass.make fuzz-short FUZZTIME=15s— all 7 targets exceptFuzzAccountDeserializepass; that one fails as documented above.Test plan
go test.FuzzMerkleHashTree.Refs #4832