test: add Go fuzz targets and fix the panics they uncovered#2541
Open
test: add Go fuzz targets and fix the panics they uncovered#2541
Conversation
google.golang.org/grpc v1.79.3 absorbed the stats/opentelemetry sub-package, but it is still pulled in transitively (via helm.sh/helm/v3 -> distribution/distribution/v3@v3.0.0) as a separate module. This double-import makes `go test ./...` fail with an "ambiguous import" error. Adding an `exclude` directive for the standalone module forces all imports of google.golang.org/grpc/stats/opentelemetry to resolve to the absorbed sub-package. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…cking splitFileName parses "(epoch:)name-version-release.arch" RPM file names by walking strings.LastIndex calls. Two edge cases fed by `dnf` / `yum`-style outputs caused incorrect behavior: 1. When the colon separator appears past the version boundary (e.g. "NetworkManager-1:1.40.16-1.el8.x86_64", emitted by some dnf builds where the epoch is baked into the version field instead of being a leading prefix), `basename[epochIndex+1:verIndex]` inverted and panicked with `slice bounds out of range`. Treat the colon as an epoch separator only when it sits before the detected version index; otherwise fall back to no-epoch parsing. 2. Inputs like "--.0" satisfied the existing `archIndex` / `relIndex` / `verIndex` guards but produced an empty Name silently. Surface this as the same "unexpected file name" error the early guards already return so callers do not insert a Package with an empty Name. Both were found by the new FuzzSplitFileName target. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two pkg-audit / pkg-version output parsers indexed strings.Fields results without checking length, panicking on truncated lines: - parseBlock at "CVE:" (no value): `strings.Fields(l)[1]` panicked with index out of range. Skip the line when the CVE id is missing instead. - parsePkgVersion at lines like "name <" with the "<" status but fewer than 7 whitespace-separated fields: `fields[6]` panicked. The expected format is `<name>-<ver> < needs updating (index has <new>)`, so guard with `len(fields) < 7`. Both were found by the new FuzzParseBlock and FuzzParsePkgVersion targets. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
github.com/knqyf263/go-cpe panics on some malformed CPE strings (e.g. "cpe:/a:vendor:product:1\:2" with embedded backslash-escapes) rather than returning an error. A single bad CPE in user config should never bring the whole process down; recover the panic and surface it as a normal error so the caller's xerrors-wrapping path handles it. Found by the new FuzzToCpeURI target. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The non-Inc. Juniper branch in Convert calls `strings.Fields(v)[0]` on the substring after " version ". When SysDescr0 ends with "version " (no value follows), Fields returns an empty slice and the indexing panicked. Skip the screenos CPE generation when no version token is present. Found by the new FuzzConvert target. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds 11 fuzz functions exercising parsers most exposed to externally controlled input. This raises the OpenSSF Scorecard "Fuzzing" check (it requires only one Fuzz* function in the repo) and gives a regression gate for the panic / logic-bug fixes that motivated this PR. Targets and seeds (existing unit-test inputs are reused as seed corpus where applicable): detector/library FuzzCanonicalMavenPURL scanner/redhatbase FuzzSplitFileName, FuzzParseInstalledPackagesLine scanner/debian FuzzParseScannedPackagesLine scanner/alpine FuzzParseApkIndex scanner/freebsd FuzzParsePkgVersion, FuzzParseBlock scanner/suse FuzzParseOSRelease config FuzzToCpeURI gost FuzzParseCwe contrib/snmp2cpe/pkg/cpe FuzzConvert Each target asserts: never panic; if err == nil, hold a small set of output invariants (e.g. map key matches Name, no character invention, non-empty mandatory fields). Combined ~15s smoke run per target gives ~1.5M+ executions and surfaced the six bugs fixed in the preceding commits. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds native Go fuzz targets across several parser-heavy packages and hardens the corresponding parsing code paths against malformed input. It fits into the codebase as a reliability-focused change set: improving parser robustness, preventing panics in config/package parsing, and enabling repository fuzzing coverage for scorecard/testing goals.
Changes:
- Add new
Fuzz*targets for parser helpers inscanner,detector,gost,config, andcontrib/snmp2cpe. - Fix panic/guard conditions in Red Hat RPM parsing, FreeBSD parsing, CPE normalization, and SNMP-to-CPE conversion.
- Add a
go.modexclude directive to avoid thegrpc/stats/opentelemetryambiguous import during builds.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
scanner/suse_fuzz_test.go |
Adds fuzz coverage for SUSE /etc/os-release parsing invariants. |
scanner/redhatbase_fuzz_test.go |
Adds fuzz targets for Red Hat installed-package parsing and RPM filename splitting. |
scanner/redhatbase.go |
Hardens splitFileName against panic cases and empty-name outputs. |
scanner/freebsd_fuzz_test.go |
Adds fuzz targets for FreeBSD pkg version and pkg-audit block parsing. |
scanner/freebsd.go |
Adds bounds checks to avoid panics on truncated FreeBSD parser input. |
scanner/debian_fuzz_test.go |
Adds fuzz coverage for Debian scanned-package line parsing. |
scanner/alpine_fuzz_test.go |
Adds fuzz coverage for Alpine APKINDEX parsing. |
gost/redhat_fuzz_test.go |
Adds fuzz coverage for Red Hat CWE string parsing. |
go.mod |
Excludes the standalone gRPC OpenTelemetry module to resolve ambiguous imports. |
detector/library_fuzz_test.go |
Adds fuzz coverage for Maven PURL canonicalization. |
contrib/snmp2cpe/pkg/cpe/cpe_fuzz_test.go |
Adds fuzz coverage for SNMP result to CPE conversion. |
contrib/snmp2cpe/pkg/cpe/cpe.go |
Guards Juniper ScreenOS parsing against missing version tokens. |
config/tomlloader_fuzz_test.go |
Adds fuzz coverage for CPE URI normalization. |
config/tomlloader.go |
Recovers from third-party CPE parsing panics and returns errors instead. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+744
to
750
| if epochIndex != -1 && epochIndex < verIndex { | ||
| epoch = basename[:epochIndex] | ||
| } else { | ||
| epochIndex = -1 | ||
| } | ||
|
|
||
| name = basename[epochIndex+1 : verIndex] |
Comment on lines
+71
to
+74
| vfields := strings.Fields(v) | ||
| if len(vfields) == 0 { | ||
| break | ||
| } |
| @@ -0,0 +1,44 @@ | |||
| package gost | |||
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.
Summary
Introduces native Go fuzz tests (
go test -fuzz) for 11 parser functions across the codebase, and ships fixes for the six panics / logic bugs the fuzzer found while exercising them. This also satisfies the OpenSSF Scorecard Fuzzing check, which currently scores 0 because noFuzz*(*testing.F)functions exist in the repo (https://scorecard.dev/viewer/?uri=github.com%2Ffuture-architect%2Fvuls).New fuzz targets
detectorcanonicalMavenPURLFuzzCanonicalMavenPURLscanner(redhatbase)splitFileName,parseInstalledPackagesLineFuzzSplitFileName,FuzzParseInstalledPackagesLinescanner(debian)parseScannedPackagesLineFuzzParseScannedPackagesLinescanner(alpine)parseApkIndexFuzzParseApkIndexscanner(freebsd)parsePkgVersion,parseBlockFuzzParsePkgVersion,FuzzParseBlockscanner(suse)parseOSReleaseFuzzParseOSReleaseconfigtoCpeURIFuzzToCpeURIgostparseCweFuzzParseCwecontrib/snmp2cpe/pkg/cpeConvertFuzzConvertEach target seeds the fuzzer with the existing unit-test inputs and asserts: never panic; if
err == nil, hold a small set of output invariants (e.g. map key matchesName, no character invention, non-empty mandatory fields).A 15-second smoke run per target totals ~1.5M+ executions and is enough to surface the six bugs below.
Bugs found and fixed
scanner/redhatbase.splitFileName— slice bounds panic. Input likeNetworkManager-1:1.40.16-1.el8.x86_64.rpm(epoch colon embedded in the version field, as somednfbuilds emit) inverted thebasename[epochIndex+1:verIndex]slice and crashed. Treat the colon as an epoch separator only when it sits before the detected version index.scanner/redhatbase.splitFileName— silent empty Name. Inputs like--.0returnednilerror withName="". Surface this as the sameunexpected file nameerror the early guards already emit.scanner/freebsd.parseBlock—Fields(l)[1]panic. A bareCVE:line (no value) crashed; skip the line instead.scanner/freebsd.parsePkgVersion—fields[6]panic. A line likename <\n(the<status with fewer than the 7 expected whitespace-separated fields) crashed; guard withlen(fields) < 7.config.toCpeURI— third-party panic.github.com/knqyf263/go-cpe'sBindToURIpanics on inputs likecpe:/a:vendor:product:1\:2. Recover and surface the panic as a normal error so a bad CPE in user config can never bring the process down.contrib/snmp2cpe/pkg/cpe.Convert—Fields(v)[0]panic. ASysDescr0ending inversion(no value) crashed the non-Inc. Juniper branch; skip screenos CPE generation when no version token is present.Build environment
The first commit is a build prerequisite:
go test ./...currently fails onmasterwith an ambiguous import error becausegoogle.golang.org/grpc/stats/opentelemetryis pulled in twice (once absorbed intogoogle.golang.org/grpc v1.79.3, once as a standalone module brought in transitively byhelm.sh/helm/v3 → distribution/distribution/v3@v3.0.0). Anexcludedirective ingo.modresolves the conflict without a dependency upgrade.Test plan
go build ./cmd/...(withGOEXPERIMENT=jsonv2)go build -tags scanner ./cmd/scanner/(withGOEXPERIMENT=jsonv2)go test ./detector/ ./scanner/ ./gost/ ./config/ ./contrib/snmp2cpe/pkg/cpe/— all PASSgolangci-lint run— only one pre-existingpreallocfinding, unrelated to this PRFuzz*target re-run for 15s post-fix — all PASS, ~1.5M+ executions totaldnf/yumoutputs (the changed behavior only triggers when the colon sits past the version boundary)🤖 Generated with Claude Code