Repository navigation
fix: harden scanning, extraction, diffing, and rendering - #1779
Merged
Merged
Conversation
mal diff --report dereferenced nil on an empty report entry or behavior, and the deferred os.Exit in main hid the panic behind exit 0. Fixing it led to an audit of the report loader, diff, renderers, CLI, scan pipeline, archive extraction, OCI handling, and detection engine. - CLI: panics exit 2 with a trace; matched exit conditions exit 1 and render the report; missing scan paths and usage errors exit 22. - Detection: scan mode applies its threshold after overrides and ignores; overrides apply per rule before dedup; --ignore-self matches the running binary by SHA-256; evidence keeps escaped bytes. - Scanning: per-file errors no longer abort a scan; .git is scanned except objects; --processes reads /proc/PID/exe; FIFOs no longer block the scan. - Archives: format comes from content; deb control members extract; bad entries are skipped; caps cover the nested tree. - OCI: layers apply as containerd applies them; every platform of an index is scanned; the image config is scanned; credentials refuse plain HTTP. - Diff and rendering: risk filters work at the default sensitivity; report text is escaped in every renderer; report.Load rejects damaged reports and loads YAML in linear time. - Dependencies: crypto/sha256 replaces minio/sha256-simd, go.yaml.in/yaml/v3 replaces gopkg.in/yaml.v3, and OrderedFileReports replaces wk8/go-ordered-map.
egibs
force-pushed
the
fix/diff-report-empty-entries
branch
from
October 27, 2026 19:43
fec3bbf to
4eb03e8
Compare
egibs
enabled auto-merge (squash)
October 27, 2026 19:44
mattlorimor
approved these changes
Oct 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This started with a crash.
mal diff --reportdereferenced nil when a report held an empty entry or behavior, and the CLI's deferredos.Exitinmainhid it: the process exited 0 with no output. Fixing that led to an audit of the report loader, diff, renderers, CLI, scan pipeline, archive extraction, OCI handling, and detection engine. This PR fixes what the audit found. A test that failed before its fix confirms each finding.Behavior changes
CLI
os.Exitturned it into exit 0.--exit-first-hitand--exit-first-missexit 1 and still render the report. Before, they exited 0, and JSON and YAML wrote nothing.-compatpackage holds, is still skipped.--sensitivityoutside 1 to 5 exit 22. Before, an unknown command made urfave/cli exit 3, the profiler-error code, after-owas truncated.--format interactivewith-ois rejected;--statswrites to-o; a second Ctrl-C ends the process at once.Detection
--ignore-selfskips only the running binary, matched by SHA-256. Before, any file namedmalwith three package paths in it was skipped.--quantity-increases-riskcounts reported HIGH behaviors, not raw matches.\x7fELF) instead of pattern IDs, capped at 1,000 strings per behavior;MatchStringsOmittedcounts the rest.Scanning
.gitis scanned except.git/objects, so hooks are no longer invisible.--processesreads/proc/PID/exe, so deleted binaries are scanned and an argv[0] decoy is not. Interpreter scripts are scanned too.--max-filesbounds walked files.Archives
.apk,.vsix, or zip with any name gets extracted.control/.malcontent-metadata.binsidecar.OCI images
--platformnarrows the set.malcontent-image-config.sh.--oci-allow-insecure-authis set, and registry hosts are normalized.--oci-pull-timeout-secondsis a response-header timeout. The new--oci-pull-deadline-seconds, default 3600, bounds the whole pull.--oci-retry-max-attemptsis the real total.Diff
--file-risk-changeand--file-risk-increasework at the default sensitivity.Rendering
\uescapes.Report loading
report.Loadrejects damaged reports: empty entries or behaviors, non-finite floats, unknown risk scores, and diff reports.API changes
DiffReportfields are*malcontent.OrderedFileReports, replacingwk8/go-ordered-map.report.Generateandreport.GenerateRulesdrop their unused logger and risk parameters.report.ExtractImageURIandarchive.GzMIME, which had no callers left.Behavior.MatchStringsOmitted,file.InspectContents,file.MapContents,file.Inode,archive.ErrUnsupportedArchive,programkind.FileType.Archive, and theOCIPlatforms,OCIAllowInsecureAuth, andOCIPullDeadlineSecondsconfig fields.ScanResult's YAML tag isfiles, matching the renderer (fix: load YAML reports in diff --report #1769 review).Dependencies
crypto/sha256replacesminio/sha256-simd, whose last commit was 2023-02-22. The stdlib takes the same SHA-NI and ARMv8 SHA2 paths and was equal or faster in a benchmark on Apple silicon: 913 to 1077 MB/s against 786 to 802 at 64 bytes, and level at 4 KiB and 1 MiB.go.yaml.in/yaml/v3v3.0.5 replacesgopkg.in/yaml.v3, archived 2025-04-01.OrderedFileReportsreplaceswk8/go-ordered-map/v2, which has not merged a change since 2025-04-01 and ties YAML encoding togopkg.in/yaml.v3. JSON and YAML output is unchanged byte for byte.go.modloses ten modules in all.cosnicolaou/pbzip2andklauspost/compress. On 32 MiB corpora, pbzip2 decoded about 10 times faster thancompress/bzip2, and klauspost gzip 1.5 to 1.8 times faster thancompress/gzip. The threepaniccalls in pbzip2 v1.0.6 guard invariants no input reaches;pkg/archive/decoders.gocites the checks.Rules
trino_upx_overridelowersupx_elf,linux_s390xandlinux_sparc64lowerpossible_dropper, andsecurity_solution_pluginlowershttp_url_with_powershell. Applying overrides per rule exposed these siblings on known-clean samples.fchmodismedium. Its tag was the typonotamble, meant asnotable.net/ip/ip-spoof.yarais nowspoof-tool.yara, andnet/proxy/tunnel.yarais nowtunnel-create.yara. Their behavior IDs collided with sibling files, so dedup dropped one of each pair.filetypesin four rules and one override became extensions, so those rules fire again; a corpus test rejects unknown values.update.shnow clears each feed before copying.unsatisfiable_exprorunintended_pattern_in_setwarnings; 35 current third-party rules are allowlisted for triage.Test data
tests/goldens were regenerated withmal refreshincgr.dev/chainguard/wolfi-base(amd64, Go 1.27.3). Wolfi ships yara-x 1.22.1, and the Makefile pins 1.22.0.pkg/action/testdatagoldens were regenerated with a new-updateflag.Known limits
python3.11/andpython3.12/, pair only when the two trees share no other key.Verification
make verify: go vet, gosec, gofumpt, golangci-lint v2.15.0, yara-x fmt, andgo test -race ./pkg/... ./internal/.... Run on macOS withGOFLAGS=-p=1.go fix -omitzero=false -diff ./...and the modernize analyzer: no changes.map[string]boolwhosefalsevalue means "listed for removal"./proc, cgroups, pseudo filesystems) were tested incgr.dev/chainguard/go.