Skip to content

fix: stopped cleanup from removing types needed by other candidate types - #248

Closed
agski331 wants to merge 2 commits into
BinFlip:developfrom
agski331:fix/removing_needed_types
Closed

fix: stopped cleanup from removing types needed by other candidate types#248
agski331 wants to merge 2 commits into
BinFlip:developfrom
agski331:fix/removing_needed_types

Conversation

@agski331

Copy link
Copy Markdown

Currently, my understanding of the unreferenced type finder in dotscope/src/cilassembly/cleanup/analysis.rs find_unreferenced_types() is:
Searches for candidate types, with the following criteria:

  • Is not the type, is not public
  • Is not an entry point type
  • is not already tagged for deletion
  • Has at least one non .cctor method
    Each candidate is checked for a few criteria to determine if it is externally called:
  • Not already a deleted method
  • Has a caller type
  • A caller for the type is not a candidate, but the callee type is

This logic seems to work well, however it does not account for candidate types that contain methods which reference other candidate types. Because the caller is a candidate, these types cannot be inserted into has_external_caller. This causes potential issues in situations where a candidate type is determined to be externally referenced, and thus no longer a candidate, but other candidate types that are still present in candidates which are only referenced by other candidate types which may be referenced by an entrypoint type are still considered candidates for removal.

Any confuserex obfuscated binary will trip this issue, as it contains multiple classes under the definition that are not public and thus considered candidates for removal, but are only referenced by other candidates. The end result is a significant amount of valid code being considered dead by later parts of the pipeline and most of the executable's constant protection infrastructure methods being removed, despite the executable not being successfully deobfuscated by any other stage, and thus still present in the executable.

I propose adding a while loop to the code, while also removing any type from candidates that is determined to be referenced by an external type. The while loop can track changes in candidates, and once there is a pass with no change in candidates or all candidates are determined valid, execution can proceed.

BinFlip and others added 2 commits July 17, 2026 05:20
### Changed

- **Dependencies**: bumped `analyssa` (0.2.0 → 0.3.0), `num-bigint`
(0.5.0 → 0.5.1), `tokio` (1.52.3 → 1.52.4), `clap` (4.6.1 → 4.6.2),
`anyhow` (1.0.102 → 1.0.103), and `env_logger` in `dotscope-cli`
(0.11.10 → 0.11.11, aligning it with the version `dotscope` already
used)

This is a patch release: dotscope's own public API is unchanged. `SsaOp`
is not re-exported, `conv_op_for_target` is crate-internal, and the
analyssa types dotscope *does* re-export (`BinaryOpKind`, `CmpKind`,
`UnaryOpKind`, `PhiNode`, `PhiOperand`, `Target`, `PointerSize`, and the
loop/symbolic/dataflow types) are all unchanged in analyssa 0.3.0.

Most of analyssa 0.3.0's correctness fixes target native lifters and do
not apply to the CIL frontend: the `ld2r`/`setffr`/`dmb` effect
corrections concern AArch64/x86 ops dotscope never emits, and the GVN
`ComputeFlags`/`CallClobber` fixes concern native flag and call-clobber
markers with no CIL equivalent. Likewise the pointer-conversion
signedness fix has no observable effect here — dotscope never emits
`PtrToInt`, and `CilTarget::convert_const` declines pointer targets, so
`IntToPtr` does not constant-fold. What dotscope does inherit is
analyssa's structural GVN value key (replacing a per-candidate
`Debug`-string key) and the `SsaEditor::nop_instruction` fix that no
longer leaves a dead `result_type` on a removed instruction.

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@codecov

codecov Bot commented Jul 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@BinFlip
BinFlip changed the base branch from master to develop July 27, 2026 02:34
@BinFlip

BinFlip commented Jul 27, 2026

Copy link
Copy Markdown
Owner

Merging this via #250 — your commit is included as the first of the series, unchanged and with your authorship, so it lands as yours.

The reason I couldn't merge this PR directly: your branch is based on master, and my release history means master and develop had diverged, so retargeting produced conflicts across 17 files that had nothing to do with your change. I'm fixing it at the source (#250 also moves publishing off push-to-master, and develop is going away).

Your diagnosis was right, and #250 builds on it — including the nested-type case you flagged, which turned out to matter for your own sample. The sample also surfaced a chain of constant-decryption bugs; details in #249.

Thanks for both the patch and the sample!

@BinFlip BinFlip closed this Jul 27, 2026
@BinFlip BinFlip mentioned this pull request Jul 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants