ci: make the fuzzing, coverage and perf gates catch things they were missing - #226
Open
r0ny123 wants to merge 13 commits into
Open
ci: make the fuzzing, coverage and perf gates catch things they were missing#226r0ny123 wants to merge 13 commits into
r0ny123 wants to merge 13 commits into
Conversation
r0ny123
force-pushed
the
smda-maturity-assessment
branch
from
August 5, 2026 20:16
c4c220e to
bc43027
Compare
end-of-file-fixer appended a newline to an obfuscated fuzz corpus entry the first time one was committed, corrupting it rather than tidying it. The same hazard already applied to the XOR-obfuscated malware fixtures and to tests/fuzz_regressions/; only the ApiScout databases were excluded. The exclude now covers all four, while still linting the READMEs inside them.
Three distinct ways ElfFileLoader.getAbi and the Mach-O readers broke against current LIEF. getAbi caught lief.bad_file, and that attribute no longer exists in lief 0.17 - hasattr reports False - so the moment the handler was needed it would itself have raised AttributeError. It also read .name off identity_os_abi, but LIEF returns a plain int for an OS ABI it has no enum member for, and .name on an int is another AttributeError. Now probes for the name and catches ValueError, naming no LIEF symbol at all so it survives further drift. Deliberately not widened to `except Exception`: AttributeError and TypeError here are the defect classes the fuzzing oracle exists to surface. Separately, both Mach-O readers already handle an unknown CPU type, but LIEF's RuntimeWarning still propagated, so a consumer running under -W error saw a crash on a malformed header instead of the unsupported metadata the code returns. That also matters now the suite runs with filterwarnings=error.
Two defects on the SmdaReport.fromDict path, both invisible to the test suite and both reachable by any consumer importing a report - MCRIT included. Validation. fromDict read fields unconditionally, so a truncated report surfaced as a KeyError from somewhere mid-rebuild rather than the ValueError the surrounding validation uses, and nothing constrained the values that did arrive. The fuzzer found four separate ways through, each one narrower than the last, so this lands as a single boundary rather than a pile of point guards: required fields must be present; containers must be containers; addresses and sizes must be integers rather than any number, since struct.pack_into rejects a float outright; and every address - report scalars, xcfg keys, function offsets, block addresses - must fit the 64-bit space the synthesizers pack them into. The xcfg key was the subtle one. It becomes the function offset via int(function_addr) and had no sign check, so a negative key reached an ELF program header as p_vaddr = -14200832. It also slipped past the image-span bound, because that measures max - min and a negative floor under a positive ceiling still measured a legal 17 MB. The address check deliberately sits after the container-shape checks. Putting it first made every subtest in test_function_rejects_malformed_container_fields short-circuit on the null offset of a bare SmdaFunction, so they kept passing while no longer testing the field each one names - a regression only the diff-coverage gate surfaced. Nesting depth. SmdaFunction.fromDict passed _calculateNestingDepth() as a dict.get() default, and Python evaluates that whether or not the key is present, so every function on the cached path built a full dominator tree and discarded it. Now an explicit branch: verified 0 calls with the field present, 1 without.
src/smda/synthesis/ writes binary headers from report-derived integers and is documented as not hardened against adversarial input. The new fuzz target found three ways to reach struct.pack with a value it cannot encode. An inverted segment span. The synthetic section covering functions outside every known section took its floor from function offsets and its ceiling from function extent ends - independent quantities, aligned at different granularities (down to PAGE_SIZE, up to 16). Real binaries always have an extent end above the offset it started from; a report whose blocks sit below their own function offset does not, and struct.pack got -4080 for an unsigned field. The sweep found the same shape at four sites, MachoSynthesizer twice and ElfSynthesizer twice, of which only one had crashed; all four now share _syntheticSpan(). PeSynthesizer has the shape but is benign, its ceiling always clears the floor. Unbounded allocation. SmdaConfig.MAX_IMAGE_SIZE has been 100 MB all along and every loader enforces it, but the synthesizers - which build images rather than load them - never got the same guard, so functions 1 GB apart produced a 1 GB bytearray. Repeated across fuzzing iterations that is what walked RSS past libFuzzer's limit. The check goes in _resolveFunctionOffsets, the one path all three formats share. A derived entry point. base_addr and oep can each sit inside the address space while their sum does not, so ElfSynthesizer keeps its own guard: that value comes from the report indirectly, and the deserialization bound structurally cannot see it.
Not a synthesis bug despite the target that found it: escapeBinaryPtrRef is reached from SmdaReport.fromDict via getPicHash, so any consumer importing a report can hit it. The operand regex accepts a hex literal of any length. Both escapeBinaryPtrRef and escapeBinaryValue pack it as "<I" and fall back to "<Q" on struct.error, with nothing behind that second attempt, so a value at or above 2**64 raises out of the fallback itself. Both now return the sequence unescaped - a displacement that wide cannot appear in the instruction bytes, so there is nothing to wildcard. Masking the value to the pack width would read cleaner but changes pic_hash for reports that work today, and pic_hash is a compatibility surface.
SmdaConfig is class-attribute based, so API_COLLECTION_FILES was one dict shared by every instance in the process: one config registering an ApiScout database wrote through to the class and leaked into every other config alive alongside it. That is precisely SMDA's batch and service deployment shape. The package also installed no NullHandler, so a consumer that never configures logging got handler-of-last-resort output on stderr. tests/testPackageMetadata.py additionally pins the two hand-maintained version strings together - they are written into every report and drive the recalculate-on-import decisions, with nothing previously checking they agree.
Eight methods - addGapCandidate, addTailcallCandidate, addReferenceCandidate, addLanguageSpecCandidate, addPrologueCandidate, addSymbolCandidate, addExceptionCandidate and locateLangSpecCandidates - re-declared their own parent's body character for character. This is the copy-paste-between-arch-families shape that sibling_check.py exists to catch in a diff, sitting in the source instead. Identified by comparing parsed bodies, not by eye: six other overrides in the same class (__init__, init, nextGapCandidate, checkFunctionOverlap, ensureCandidate, locateCandidates) genuinely differ and are untouched. Dispatch is on self, so a parent body calling back into the subclass still reaches the intel override. Verified output-identical: report identity hashes, function counts, block counts and instruction counts across cutwail, asprox, bashlite, komplex, mirai_i386 and mirai_x64 are unchanged. Unifying the FunctionAnalysisState hierarchy is a real refactor and is deliberately not part of this.
A previous commit switched six writes in evaluate_runtime.py to explicit utf-8 and left every matching read alone, including the cache file it had just started writing as utf-8. On Windows those reads use cp1252 and fail on any non-ascii byte, which the emoji-bearing markdown these scripts emit guarantees. Swept the class rather than the instance: the remaining benchmark-script sites in both directions, SmdaReport's toFile/fromFile, BatchProcessor's per-sample report writes, WinApiResolver's ApiScout database read, and the fixture manifests the tests read. Report content is caller-supplied - filenames, symbol names - so none of it is safe to leave locale-dependent now the test matrix includes windows-latest. Note the reads matter more than the writes here: json.dump defaults to ensure_ascii=True, so SMDA's own report files are pure ascii. A report written by another tool, or the markdown these scripts emit, is not.
…s floor Three changes to the fuzzing setup that only make sense together. The oracle was permissive. ExceptionHandling's operational/non-operational split is the library contract - degrade gracefully for the caller - and fuzzing/targets.py added only RecursionError on top, so TypeError, AttributeError, KeyError, IndexError, struct.error and friends were swallowed. Those are precisely how most real Python defects present. The stricter oracle lives in fuzzing/, not in src/, so the library contract is untouched, with a narrow documented allowlist. A synthesis target. src/smda/synthesis/ is ~1.9k lines writing binary headers from report-derived integers, documented as unhardened, with 14 tests and no fuzzing. The target runs a fuzzer-built report through synthesizeBinary() for each format and asserts the bytes re-parse with LIEF - a synthesizer whose output no longer loads has failed at the one thing it is for. It found every synthesis and escaper bug in this branch. A committed corpus floor. The accumulating corpus lived only in the Actions cache, which evicts after 7 days without a hit, so a quiet fortnight silently reset coverage progress to bare seeds. tests/fuzz_corpus/ now holds a 212 KiB pruned floor, XOR-obfuscated with the same scheme as the malware fixtures because most entries derive from them; the cache stays the accumulating layer on top. tests/fuzz_regressions/ had a documented convention and zero entries. It now holds the reproducers for the bugs found here, each verified to bisect its fix rather than merely pass.
Three gates that were either absent or reporting success without checking. Coverage was configured but never measured: branch coverage and an exclude list existed, with no fail_under, no --cov anywhere in .github/, and ignore_errors silently swallowing measurement errors. Nobody knew the number. It is now collected on one matrix leg, published to the step summary, floored at 75 (measured 81), and gated on diff coverage - a global floor on 24.7k lines barely moves when a new module lands untested, which is the case that matters. The omit glob *lib* is narrowed to */lib/*. Tests ran on Linux only, across all four Python versions; Windows and macOS appeared only in a job that imports smda and prints versions. For a library that parses PE and Mach-O, platform breakage would ship unnoticed. Both now run the real suite on the 3.11 leg. They passed first time, which the utf-8 sweep in this branch is what made true. Supply chain: amannn/action-semantic-pull-request ran from a floating tag under pull_request_target, i.e. with write-capable context, while every other action was SHA-pinned. Pinned. A Security workflow adds pip-audit against the dependency set resolved from pyproject - not the runner's environment, which drags in unrelated tooling - and zizmor over the workflows, both on a weekly cron because a dependency CVE lands without anything here changing. zizmor's one finding is suppressed inline with a reason: that workflow never checks out or executes PR content. pip-audit immediately earned its place. The setuptools bound in the Makefile and README pinned contributors below the fix for PYSEC-2026-3447, which landed in 83.0.0; pyproject already allowed it. Aligned. SECURITY.md did not exist for a library whose entire purpose is parsing hostile input.
The perf gate failed only at a 50% slowdown, so four consecutive 15% regressions could land green. There was also no memory channel at all, despite the README documenting ~1.8 GB peak for a 3 MB binary and batch mode sizing its worker count by per-file peak - the resource most likely to break a downstream consumer was unmeasured. run_perf_check.py now records peak allocation via tracemalloc, in its own pass so tracing overhead cannot distort the timing channel beside it, and merge_results accumulates it across counterbalanced passes like the timing samples. compare_perf gates it at 10%: allocation totals barely vary run to run, so it can be far tighter than wall-clock. Lowering the time threshold to 30% then made the gate fire on its first real PR: komplex went 0.2148s -> 0.2829s, +31.7%. It was noise - the overall row moved -0.9%, three fixtures improved 6-11%, every report hash matched, and the new memory channel showed komplex allocating 10.3 MiB on both sides, +0.01%. A fixture doing a third more work moves its allocation; this one did not. The counterbalanced passes cancel drift, not per-sample variance on a 0.2s workload. So a fixture must now breach the percentage and lose at least 0.15s before it fails; below that it reports WARN (NOISE?). The overall row keeps no floor, so a real across-the-board regression still fails.
An installed smda handed consumers - MCRIT included - zero types despite CI type-checking the source. src/smda/py.typed now ships (verified present in the built wheel) and the public entry point in Disassembler.py is annotated explicitly. Most of the remaining surface is inferred rather than annotated; the model classes are a deliberate follow-up rather than a broad sweep. ty now also checks fuzzing/, profiling/ and .github/workflows/scripts/ - the last of which holds evaluate_runtime.py, 955 lines of statistics gating the perf benchmark that nothing but its own tests had ever checked. profiling/ needs an unresolved-import override because its imports live in the optional profile extra, which the lint job deliberately does not install. tests/ is left out on purpose: it carries ~187 diagnostics and needs its own pass. Ergonomics: filterwarnings=error, so a DeprecationWarning from lief or capstone on 3.14 stops passing silently; a Hypothesis profile with deadline=None, since the default 200ms wall-clock deadline against max_examples=500 fails correct examples on a loaded runner; and a slow marker on the corpus-driven modules, which turns a ~150s suite into a ~15s subset for iteration.
The release history had forked. README carried 87 entries and version_history.md carried 119, and they were not a current-vs-stale pair: 71 versions appeared only in README, 94 only in version_history, 15 in both. CHANGELOG.md merges them into 188 entries newest-first with README's wording winning on overlap, README keeps a pointer, and version_history.md is gone. AGENTS.md's release procedure is updated to match, since it directed editing the README section that no longer exists. Also in AGENTS.md: it claimed a Python 3.10 target while pyproject is 3.11, and told agents to run tests through .devcontainer/run.sh, which is git-ignored and absent. `make clean` never deleted anything - its regex has an unclosed group, so grep errored out every time, and it piped to unquoted xargs. Replaced with a find -exec that survives paths with spaces. requirements.txt was consumed by nothing, omitted hypothesis, ty and diff-cover, and could only drift further; removed in favour of the dev extra. And a stray acute accent sat alone on a line in the README.
r0ny123
force-pushed
the
smda-maturity-assessment
branch
from
August 6, 2026 08:56
f2c9fde to
fef7824
Compare
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
1 participant
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.
This started as a look at where SMDA's tooling was reporting success without actually checking anything. Tightening the gates then surfaced a run of real bugs, several of them on the report-import path that MCRIT uses.
13 commits, one per area, each self-contained — no file appears in two of them, so it should read commit by commit if that's easier than the combined diff. Happy to split it up if you'd rather take it in pieces; a few parts are opinionated and I've flagged those at the bottom.
Bugs fixed
lief.bad_filedoesn't exist any more.ElfFileLoader.getAbicaught it, and it's gone in lief 0.17 (hasattrsays False) — so the moment that handler was needed it would itself have raisedAttributeError. It also read.nameoffidentity_os_abi, but lief returns a plain int for an ABI it has no enum member for, and.nameon an int is anotherAttributeError.Report deserialization accepted almost anything.
fromDictread fields unconditionally, so a truncated report surfaced as aKeyErrorfrom mid-rebuild rather than theValueErrorthe surrounding validation uses, and nothing constrained the values that did arrive. Four separate ways through, all now closed at one boundary: required fields present, containers actually containers, addresses and sizes integers rather than any number (struct.pack_intorejects a float outright), and every address inside the 64-bit space the synthesizers pack it into.The subtle one was the xcfg key — it becomes the function offset via
int(function_addr)with no sign check, so a negative key reached an ELF program header asp_vaddr = -14200832. It slipped past the image-span bound too, because that measuresmax − minand a negative floor under a positive ceiling still measured a legal 17 MB.A cached nesting depth was recomputed for every function:
Python evaluates a
.get()default whether or not the key is there, so every function on the cached path built a full dominator tree and discarded it. Verified by counting calls: 1 with the field present, should be 0.Synthesis could build an inverted segment. The synthetic section for functions outside every known section took its floor from function offsets and its ceiling from extent ends — independent quantities at different granularities. Real binaries always have an extent end above the offset it started from; a crafted report doesn't, and
struct.packgot-4080unsigned. Same shape at four sites, only one of which had crashed.Synthesis allocated without bound.
MAX_IMAGE_SIZEhas been 100 MB all along and every loader enforces it — the synthesizers, which build images, never got the same guard, so functions 1 GB apart produced a 1 GB bytearray.escapeBinaryPtrRefcouldn't survive a wide displacement. Reached fromSmdaReport.fromDictviagetPicHash, so this one is on the plain report-import path. The operand regex takes a hex literal of any length; both escape helpers pack<I, fall back to<Q, and had nothing behind the fallback.SmdaConfigleaked state between instances — class-attribute based, soconfig.API_COLLECTION_FILES[name] = pathwrote through to the class and showed up in every other config in the process. That's exactly SMDA's batch/service shape.Windows encoding. A previous commit switched six writes in
evaluate_runtime.pyto explicit UTF-8 and left every matching read alone, including the cache file it had just started writing as UTF-8. Swept the rest. The reads matter more than the writes:json.dumpdefaults toensure_ascii=True, so SMDA's own reports are pure ASCII — a report written by another tool, or the markdown these scripts emit, is not.make cleannever deleted anything — unclosed group in its regex, so grep errored out every time.Added
A synthesis fuzz target, a stricter oracle, and a committed corpus floor. The old oracle promoted only
RecursionErroron top of the library's operational/non-operational split, soTypeError,AttributeError,KeyError,IndexErrorandstruct.errorwere swallowed — precisely how most real Python defects present. The stricter oracle lives infuzzing/, notsrc/, so the library contract is untouched.src/smda/synthesis/is ~1.9k lines writing binary headers from report-derived integers, documented as unhardened, with 14 tests and no fuzzing. The new target asserts its output re-parses with LIEF — a synthesizer whose output no longer loads has failed at the one thing it's for. It found every synthesis and escaper bug above.The accumulating corpus lived only in the Actions cache, which evicts after 7 days without a hit, so a quiet fortnight silently reset coverage to bare seeds.
tests/fuzz_corpus/now holds a 212 KiB pruned floor. Andtests/fuzz_regressions/had a documented convention and zero entries — it now holds the reproducers, each verified to bisect its fix rather than merely pass.Coverage measurement and gates. Branch coverage and an exclude list existed with no
fail_under, no--covanywhere in.github/, andignore_errorsswallowing measurement errors — nobody knew the number. Now collected on one leg, published to the step summary, floored at 75 (measured 81), and gated on diff coverage, since a global floor on 24.7k lines barely moves when a new module lands untested.The suite now runs on Windows and macOS. Previously Linux-only across four Python versions, with the other platforms appearing in a job that imports
smdaand prints versions. Both passed first time — which the UTF-8 sweep is what made true.Supply-chain scanning.
pip-auditagainst the dependency set resolved frompyproject.toml, pluszizmorover the workflows, on a weekly cron. It immediately earned its place: the setuptools bound in theMakefileandREADMEpinned contributors below the fix for PYSEC-2026-3447. Also pinned the one floating action, which ran underpull_request_target. AndSECURITY.md, which didn't exist for a library whose whole job is parsing hostile input.A peak-memory channel in the perf gate, via
tracemallocin its own pass so tracing overhead can't skew the timing channel. Gated at 10%. The time threshold drops 50% → 30%, but a fixture must now also lose 0.15s absolute before failing — at 30% alone, 68 ms of variance on a 0.2 s fixture failed the build, and the new memory channel is what proved it was noise (10.3 MiB both sides, +0.01%).py.typed, so an installedsmdastops handing consumers zero types.tyalso now coversfuzzing/,profiling/and.github/workflows/scripts/— the last holdsevaluate_runtime.py, 955 lines of statistics gating the perf benchmark that nothing but its own tests had checked.Test ergonomics —
filterwarnings = ["error"], a Hypothesis profile withdeadline=None, and aslowmarker turning a ~150 s suite into a ~15 s subset for iteration.Removed
version_history.md, folded into a newCHANGELOG.md. The two had genuinely forked — 71 versions only in README's list, 94 only inversion_history.md, 15 in both. README keeps a pointer and drops from 289 to 201 lines.requirements.txt— consumed by nothing, and missinghypothesis,tyanddiff-cover.Eight methods in
intel/FunctionCandidateManager.pyre-declaring their own parent's body character for character. Compared parsed bodies rather than eyeballing: six other overrides in the same class genuinely differ and are untouched. Report identity hashes and function/block/instruction counts across six fixtures are unchanged.SmdaConfig.LOG_PATH— defined, never read. Public attribute though, so shout if you'd rather keep it.Validation
ruff check ./ruff format --check .python -m ty check src/smda/ fuzzing/ profiling/ .github/workflows/scripts/make testdiff-cover --fail-under=100synthesis+report, 0 findingsReport identity hashes on six fixtures are unchanged across the whole branch — none of this was supposed to move recovery output, and it doesn't.
Things you might want to push back on
SmdaConfig.LOG_PATHis a public-attribute removal, even though nothing reads it.ValueErrorwhere some wereKeyErrororTypeError. Anything catchingKeyErroraroundfromDictwould stop catching.CHANGELOG.mdis a docs-layout opinion more than a fix — easy to drop that commit.diff-cover --fail-under=100is strict. It only measuressrc/smda/**, so changes undertests/,fuzzing/or the CI scripts pass by not being measured; there's a note in the workflow saying so, since it reads stricter than it is.Not done
py.typedships, but onlyDisassembler's public API is annotated — the model classes are still mostly inferred (~71 of 75 public methods). Deliberately not guessing at 71 signatures underall = "error".tydoesn't covertests/yet; ~187 diagnostics there need their own pass.pytest-xdist.SmdaConfig's mutable-default leak is fixed, butlief.logging.disable()still runs as a constructor side effect and severallru_caches key on binary content. Separately, the Mach-O synthesis tests are intermittently flaky under load — they pass in isolation, and a baseline with this branch's changes stashed shows the same behaviour, so it predates this work.