Skip to content

test: add Go fuzzing targets and scheduled fuzzing CI - #5006

Merged
TomOnTime merged 1 commit into
DNSControl:mainfrom
chicks-net:fuzz-ossf-native
Oct 15, 2026
Merged

TomOnTime merged 1 commit into
DNSControl:mainfrom
chicks-net:fuzz-ossf-native

Conversation

@chicks-net

Copy link
Copy Markdown
Collaborator

Description

Follow-up to #4993 (OpenSSF Scorecard analysis) and #5001 (Token-Permissions). The Scorecard Fuzzing check is currently 0/10; this PR takes it to 10/10 via Go native fuzzing, and the fuzzing found real panics in the dnsconfig.js import path along the way.

fuzz/ package

  • FuzzConfigJSON drives the config pipeline with the JSON shape that JSON.stringify(conf) produces in pkg/js: json.Unmarshal → PostProcess → ImportRawRecords → ValidateAndNormalizeConfig → prettyzone. This covers the entire post-JS path every record takes.
  • FuzzTXTQuoted exercises the RFC1035 TXT quoting/encoding in pkg/txtutil: no panics, 255-octet chunk invariants, and ParseQuoted(EncodeQuoted(x)) round-trip stability.
  • The seed corpus runs as ordinary tests on every go test ./... (PR CI included), so crashers are permanent regression tests once committed under fuzz/testdata/fuzz/.

The JavaScript engine itself is deliberately not fuzzed: the PANIC() helper calls os.Exit(1) (killing fuzz workers), require()/glob() expose the filesystem, and timers allow infinite-loop hangs. Fuzzing the JS layer would need those to return errors instead; out of scope here.

Panics found and fixed (errors instead of crashes)

Panic Root cause Fix
missing/non-string label arg unguarded rawRec.Args[0].(string) in ImportRawRecords length + type check → error
obscure record types (e.g. TSIG) TypeToMakeRDATA[typeNum] nil map entry for types valid in StringToType but without a registered maker existence check → error
bad arg values (e.g. CAA flag 300, non-numeric string) mustbe.Uint8/16/32 etc. panic on values they cannot convert recover() → error at the js-to-Go boundary (newRecordConfigFromDnsconfigjs, runBuilder)
domain name ending with a dot (000.) panicked in SetLabel during validation PostProcess rejects it, same as NewDomainConfig
records injected via JSON records/recordsabsent RecordConfig.rdata is unexported, so such records can never be well-formed; nil-RDATA panicked in zonefile generation PostProcess rejects nil-RDATA records

All are reachable from a crafted dnsconfig.js; dnscontrol now reports them as errors instead of crashing with a stack trace. The minimized crashers are committed as regression seeds.

.github/workflows/fuzzing.yml

  • Weekly schedule (Mon 06:17 UTC) + workflow_dispatch; a matrix runs each target with -fuzztime 10m (fail-fast: false).
  • actions/cache restores the accumulating corpus from GOCACHE/fuzz between runs.
  • On failure, crashers are uploaded as artifacts for triage.
  • permissions: contents: read; all actions pinned by commit SHA (Dependabot maintains them), harden-runner audit egress, matching the scorecard.yml style.

Verification

  • gotestsum (full suite): 3547 tests pass, including the new seeds and the two committed crashers.
  • Local fuzz runs: FuzzConfigJSON 5m/918k execs and FuzzTXTQuoted 90s/467k execs, no findings after the fixes.
  • golangci-lint clean on ./fuzz/... and ./models/...; actionlint passes on the workflow; go generate/go fix/go mod tidy produce no diffs.
  • Scorecard v5.6 --local confirms: Fuzzing 10/10 — GoBuiltInFuzzer integration found: fuzz/config_pipeline_test.go:22, fuzz/txt_test.go:14.

AI-attribution

Assisted-by: OpenCode:glm-5.3

Add a fuzz/ package with two Go native fuzz targets, satisfying the
OpenSSF Scorecard Fuzzing check (verified with scorecard --local: 10/10):

- FuzzConfigJSON drives the config pipeline (json.Unmarshal ->
  PostProcess -> ImportRawRecords -> ValidateAndNormalizeConfig ->
  prettyzone) with the JSON shape that JSON.stringify(conf) produces.
- FuzzTXTQuoted exercises the RFC1035 TXT quoting/encoding in
  pkg/txtutil, including 255-octet chunk invariants.

The JavaScript engine is deliberately not fuzzed: the PANIC() helper
calls os.Exit(1) (killing fuzz workers) and require()/glob() expose
the filesystem.

Fuzzing immediately found panics in the dnsconfig.js import path,
now fixed to report errors instead of crashing dnscontrol:

- models: a rawrecord label arg that is missing or not a string
  panicked on a type assertion; a record type known to
  dnsutilv2.StringToType but without a registered maker panicked on
  a nil map entry; and the mustbe.* helpers panic on values they
  cannot convert.  These are converted to errors at the js-to-Go
  boundary (newRecordConfigFromDnsconfigjs, runBuilder).
- models: domain names ending with a dot panicked downstream in
  SetLabel; PostProcess now rejects them like NewDomainConfig does.
- models: records injected via the JSON records/recordsabsent fields
  can never be well-formed (RecordConfig.rdata is unexported) and
  panicked in zonefile generation; PostProcess now rejects them.

The minimized crashers are committed under fuzz/testdata/fuzz/ as
regression seeds, which run on every go test ./... (PR CI included).

.github/workflows/fuzzing.yml fuzzes each target weekly for 10
minutes (plus workflow_dispatch), restores the accumulating corpus
from the build cache, and uploads any crashers as artifacts.  All
actions are pinned by commit SHA for Dependabot.

Assisted-by: OpenCode:glm-5.3
@chicks-net
chicks-net requested a review from TomOnTime as a code owner October 14, 2026 21:45
@TomOnTime
TomOnTime merged commit b2cbf30 into DNSControl:main Oct 15, 2026
11 checks passed
@TomOnTime

Copy link
Copy Markdown
Collaborator

I knew it would find a bunch of bugs! Thanks for fixing them!

@chicks-net

Copy link
Copy Markdown
Collaborator Author

I knew it would find a bunch of bugs! Thanks for fixing them!

It was really less than I expected for a project of this size. If we fuzzed the underlying JavaScript runtime we'd probably end up with many more, but I don't think we're interested in tackling that in the short term.

Luckily Golang has standardized fuzzing support and it is automatically detected by OpenSSF scorecard.

TomOnTime pushed a commit that referenced this pull request Oct 20, 2026
## Problem

The scheduled `Fuzzing` workflow (added in #5006) fails immediately —
both matrix jobs (`FuzzConfigJSON`, `FuzzTXTQuoted`) die at the very
first step:

> Unable to resolve action
`step-security/harden-runner@ccd8616d44fd3846e67624a50d5aad6d37bf2f25`,
unable to find version `ccd8616d44fd3846e67624a50d5aad6d37bf2f25`

(Example failed run: chicks-net/dnscontrol run
[39257215203](https://github.com/chicks-net/dnscontrol/actions/runs/39257215203),
and the scheduled run in this repo will hit the same wall since the file
is identical.)

## Root cause

A one-character typo in the pinned commit SHA for
`step-security/harden-runner` v2.22.1 — character 38 is `f` where it
must be `d`:

```text
upstream v2.22.1 tag:  ccd8616d44fd3846e67624a50d5aad6d37bf2d25
pinned in fuzzing.yml: ccd8616d44fd3846e67624a50d5aad6d37bf2f25  ← does not exist
```

Because the SHA does not resolve, the jobs never reach the checkout, so
no fuzzing has actually run since the workflow landed.

## Fix

Correct the single character so the pin points at the real v2.22.1
commit — the same SHA already pinned correctly in `scorecard.yml` (from
#4993):

```diff
-        uses: step-security/harden-runner@ccd8616 # v2.22.1
+        uses: step-security/harden-runner@ccd8616 # v2.22.1
```

## Verification

- The SHA was verified against the upstream tag: `GET
/repos/step-security/harden-runner/git/ref/tags/v2.22.1` resolves to
`ccd8616d44fd3846e67624a50d5aad6d37bf2d25`.
- The corrected SHA is byte-identical to the pin in
`.github/workflows/scorecard.yml`, which runs fine.
- Workflow YAML still parses cleanly; the only change is the one
character inside the `uses:` value.

After merge, a `workflow_dispatch` run of **Fuzzing** should confirm
both targets actually fuzz for the full 10 minutes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants