Skip to content

Harden the parsers, gate on gosec, and fix the broken verification docs - #3

Merged
DevCop95 merged 7 commits into
mainfrom
hardening/parsers-sast-coverage
Oct 17, 2026
Merged

DevCop95 merged 7 commits into
mainfrom
hardening/parsers-sast-coverage

Conversation

@DevCop95

Copy link
Copy Markdown
Owner

Hardening pass for #2. Four commits, one per theme, all gates green locally.

What this does

Fuzzing the parsers that eat attacker-influenced bytes — FuzzRegf (registry
hive), FuzzParseRep (KDC reply), FuzzUnpack (payload envelope),
FuzzMaskRoundTrip (obfuscator) and FuzzSanitizeName (persist backend). All
five consume input this tool did not author, in a process holding an elevated
token, where a panic is an availability bug and not a test artefact. The seeds
run as ordinary tests, so the corpus is exercised on every go test; CI adds a
30s-per-target smoke run on every push and PR.

SAST, with every suppression justified — gosec flagged 47 sites. None were
exploitable, but none were documented either, so each now carries an inline
//#nosec <rule> -- <reason>: protocol-mandated DES/RC4/MD5/MD4 ([MS-SAMR],
[MS-NLMP], RFC 4757), width narrowing bounded by an on-disk format, and fixed
argv in the schtasks/powershell helpers. Two rules are excluded on the
command line with their rationale written next to them: G103 (37 hand-reviewed
unsafe sites — the tool is an unsafe/syscall bridge) and G104 (14
unchecked errors, every one a deferred handle close). File modes tightened
0755/0644 → 0700/0600.

A real bug found on the way — xorBytes divided by len(key) with no
check. No caller passes an empty key today, so it was latent, but the failure
mode is a panic inside an elevated process. It now fails closed and returns
nil, letting the AEAD reject the key size.

Coverage 38% → 44.7%, floor 35 → 42 — tests for the portable hive parser,
WipeFile (including the hard-link truncate path), the collector helpers
(xorBytes, aesGCMOpen, forceCopy, truncateForLog), the persist backend,
the opt-in insecure TLS config and the elevate error paths.

A documentation bug that broke verification — docs/SIGNING.md told
verifiers to expect the certificate identity DevCop95/obliteratus, a
repository that does not exist; the workflow runs in DevCop95/pullgoscript.
The documented command therefore failed against the real v2.0.2 release. Now
corrected, with the observed output of both the passing and the failing case.

Community files — Contributor Covenant 2.1, issue forms that route
vulnerability reports to private reporting rather than the public tracker, a PR
checklist, and the CI gates documented in CONTRIBUTING.md.

Verification

gofmt -l .                                  # empty
go vet ./...                                # clean
GOOS=linux go vet ./...                     # clean (new test files compile off Windows)
staticcheck ./...                           # clean
go test ./...                               # 11/11 ok, total 44.7%
gosec -no-fail -fmt=json -exclude=G103,G104 ./...   # issues 0, nosec 47

Cross-compile was checked with GOOS=linux go vet ./... because this machine
has no Docker or working WSL; the Linux job in CI is the real confirmation.

Known limitations, stated rather than hidden

  • Authenticode is still not done. It needs an EV certificate or Azure
    Trusted Signing account — paid and identity-verified — so it cannot be closed
    from here. docs/SIGNING.md now carries a status table saying exactly that
    instead of implying the artifacts are Windows-trusted. No release is blocked
    by its absence.
  • gosec has a blind spot. go/types rejects the bodyless EnergyFlow
    (implemented in energyflow_amd64.s), so gosec cannot build SSA for
    internal/research and its taint rules skip that package. The job asserts
    that the missing-function-body error is the only tolerated one, so any new
    type error — which would disable analysis just as silently — fails the build.
  • The new CI jobs are not yet required checks. fuzz-smoke and sast must
    be added to the main ruleset to actually block a merge.
  • The new make sast / make fuzz-check / make cover targets were not
    executed here (make is not installed on this machine); each wraps a command
    that was run individually and is quoted in CONTRIBUTING.md for anyone
    without make.

Refs #2.

…ty XOR key

gosec's default ruleset flagged 47 sites in the credential, Kerberos and parser
packages. None were exploitable, but none were documented either, so each is
now annotated inline with the reason it cannot be reached by attacker input:
protocol-mandated DES/RC4/MD5/MD4 (MS-SAMR, MS-NLMP, RFC 4757), width narrowing
inside binary parsers bounded by their on-disk format, and fixed argv in the
schtasks/powershell helpers.

xorBytes divided by len(key) without checking it. No caller passes an empty key
today, but the failure mode is a panic inside an elevated process, so it now
returns nil and lets the AEAD reject the key size instead.

Tighten the persist shim modes to 0700/0600: Windows derives the real ACL from
the parent and only honours the read-only bit, so this costs nothing and keeps
the intent explicit if the code is ever ported.
The regf hive parser, the KDC reply parser, the payload envelope reader, the
string obfuscator and the persist name sanitizer all consume bytes this tool did
not author, inside a process that holds an elevated token. They now have fuzz
targets — FuzzRegf, FuzzParseRep, FuzzUnpack, FuzzMaskRoundTrip,
FuzzSanitizeName — and the seeds run as ordinary tests, so the corpus is
exercised on every `go test` without an explicit -fuzz.

FuzzMaskRoundTrip asserts the mask is a true involution over arbitrary bytes;
FuzzSanitizeName asserts the output is idempotent, bounded and shell-safe.

Coverage 38% -> 44.7%, with tests for the portable hive parser (loadHive
rejection, subkey/path walking, resident and hex-class values), WipeFile
including the hard-link truncate path, the collector helpers (xorBytes,
aesGCMOpen, forceCopy, truncateForLog), the persist backend, the opt-in
insecure TLS config and the elevate error paths.
Two new jobs. `fuzz-smoke` runs the five parser targets for 30s each on every
push and PR: a panic in one of them is an availability bug in the console that
holds an elevated token, not a test artefact.

`sast` runs gosec with GOOS pinned to windows — on linux the scan would
silently skip the windows-only packages, which hold the entire
security-sensitive surface. Two rules are excluded on the command line: G103
(37 hand-reviewed unsafe sites; the tool is an unsafe/syscall bridge) and G104
(14 unchecked errors, every one a deferred handle close). Everything else fails
the build. gosec cannot build SSA for internal/research because go/types
rejects a bodyless function (EnergyFlow lives in energyflow_amd64.s), so the job
asserts that the missing-function-body error is the only tolerated one: a new
type error would disable analysis just as silently, and now fails too.

Raise the coverage floor 35 -> 42 to match the new whole-tree number, and add
make targets (sast, fuzz-check, cover) so all of it is reachable locally.
…nity files

docs/SIGNING.md told verifiers to expect the certificate identity
DevCop95/obliteratus. That repository does not exist — the workflow runs in
DevCop95/pullgoscript — so the documented command failed against the real
v2.0.2 release. Replaced with the actual subject and the observed output of
both the passing and the failing case, plus a note that the identity changes if
the repository is ever renamed. The README's releases link pointed at the same
wrong name.

The Authenticode section now states the remaining supply-chain gap honestly
rather than implying the artifacts are already Windows-trusted: cosign and the
SBOM are live, Authenticode is blocked on acquiring a certificate.

Add CODE_OF_CONDUCT.md (Contributor Covenant 2.1), issue forms that route
vulnerability reports to private reporting instead of the public tracker, a PR
checklist, and document the new CI gates in CONTRIBUTING.md.
setup-go does not add $(go env GOPATH)/bin to PATH, so the scan step died with
'gosec: command not found' (exit 127) after a successful install.
Job-level GOOS cross-compiled gosec itself into a PE binary (gosec.exe), which
the Linux runner then could not execute. The runner's own GOOS is what installs
a usable binary; only the scan needs to see the windows-only packages.
"0 findings" and "the scan never parsed any code" must not look the same. The
job now asserts gosec's own Stats.files is non-zero and prints files/lines/nosec
found, so a green result is auditable rather than assumed. Verified against
gosec v2.29.0 locally before pushing: files=37, lines=7194, nosec=47, found=0.
@DevCop95
DevCop95 merged commit 45401fe into main Oct 17, 2026
5 checks passed
@DevCop95
DevCop95 deleted the hardening/parsers-sast-coverage branch October 17, 2026 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant