Fix review issues from #1375 (quantumstrand correctness/regression fixes) - #1376
Merged
mr-tz merged 21 commits intoAug 19, 2026
Merged
Conversation
XOR-obfuscated headers append ' (XOR decoded with key: 0x...)' to the layout name, so is_structured_layout()'s exact 'pe'/'elf' match failed and compute_layout() silently fell back to classic static strings, losing section names, code/reloc/XOR tags, and structure annotations. Strip any parenthetical suffix before matching.
The help text says -y defaults to not running deobfuscation, but the prompt branch -y bypasses was the only place stack/tight/decoded got disabled, so a non-interactive Go/Rust run with -y ran full deobfuscation anyway (only .NET was force-disabled). -y now skips the prompt and disables deobfuscation, matching the documented default.
The key was sha256+version only, so interpreting the same bytes under a different --format (sc32 vs sc64) could serve the other variant's stack/tight/decoded results. The CLI already disables caching for non-auto formats, but API callers passing cache_dir were unprotected. Format is now part of the key, so variants never collide.
PELayout/ELFLayout mark_structures overrides only recursed into children, never applying structures_by_address to their own strings, so strings in the ELF header/program-header gap (attached to the root node) and PE header-gap strings rendered with an empty .structure. Both now annotate their own strings before recursing, matching MachOLayout.
--summary auto-disables stack/tight/decoded and --analyze-functions disables static, so the combination silently left every string type disabled and rendered an empty summary. Since the summary only covers static strings, which --analyze-functions cannot show, the two are incompatible and now raise a clear argument error.
The max_len fallback branch tested args.libs twice, so --pes without an
explicit --max-len raised ValueError('unknown extraction type'). The
second condition now checks args.pes.
Only gp.StringHashDatabase checked for a Git LFS pointer; the oss, winapi, and expert loaders failed with a confusing BadGzipFile/msgspec error on a fresh clone without git lfs pull. A shared ensure_not_lfs_pointer() helper now raises the actionable message, and gp's loader is refactored onto it.
mr-tz
reviewed
Aug 19, 2026
10 tasks
The CLI flag was already removed; Options.disable_progress remained as a permanently-False field feeding 5 progress-gating sites. Per review, drop the flag entirely: quiet now drives those gates directly.
--yes implied it enables deobfuscation, but the flag skips the prompt and defaults to not running it, which confused reviewers. --no-prompt states the intent. The dest and Options field become no_prompt (store_true).
mr-tz
reviewed
Aug 19, 2026
mr-tz
reviewed
Aug 19, 2026
compute_key(sha256, version, format='auto') so callers that only need the default variant don't have to pass it explicitly.
The interactive deobfuscation prompt (and the confusing -y/--yes / --no-prompt flag that skipped it) is removed entirely. String deobfuscation is now opt-in: a plain run defaults to static (and language) strings only, and stack/tight/decoded must be requested explicitly via --string-type. --analyze-functions requests function-level deobfuscation, so it is not disabled by the default. This resolves the silent redirect behavior from mandiant#1176: there is no implicit 'n' anymore, just an explicit default.
Undo the opt-in default from 0abe0c2 (default now runs all string types, deobfuscation included) per PR mandiant#1376 discussion — the final behavior is still being debated. The interactive deobfuscation prompt stays removed. The --summary static-only shortcut and --analyze-functions are unchanged.
mr-tz
reviewed
Aug 19, 2026
mr-tz
reviewed
Aug 19, 2026
mr-tz
reviewed
Aug 19, 2026
mr-tz
reviewed
Aug 19, 2026
mr-tz
reviewed
Aug 19, 2026
Runs serve cached documents by default, which silently mixes output from different code revisions. Document FLOSS_CACHE_ENABLE=0 for reproducing and diffing.
format='auto' is now the default, so tests no longer pass it explicitly.
Previously only non-static values were rejected, silently allowing --summary --string-type static. --summary is its own static-only view, so any --string-type/--no-string-type is now an argument error.
MAX_LEN_LIBS (64) now applies to --libs and MAX_LEN_PES (100) to --pes; they were swapped.
It only re-asserted the Analysis defaults and added a slow full run; the default all-on behavior is covered implicitly elsewhere.
The layout tree widened \t to 4 spaces while every other view escapes it to the literal two-char sequence via sanitize(). json.dumps() already escapes \t (and \n, \r), so dropping the widening makes the layout view consistent: control chars render as literals and stay on one line.
mr-tz
approved these changes
Aug 19, 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
2 participants
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.
Addresses the valid issues from #1375