diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index f0124d47..06c232eb 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -1,26 +1,54 @@ name: Release and Publish +# Publishing is driven by publishing a GitHub release, not by pushing to a +# branch. Create the release and its tag when you intend to ship; until then +# nothing reaches crates.io, so merging to any branch is always safe. on: - push: - branches: [ main, master ] - paths-ignore: - - '**.md' - - 'docs/**' - - '.github/**' - - '!.github/workflows/release.yml' + release: + types: [published] env: CARGO_TERM_COLOR: always jobs: - # First ensure all tests pass + # Derive the version from the release tag and hold it against Cargo.toml, so a + # mistyped tag fails here rather than shipping under the wrong version. + version: + name: Resolve Version + runs-on: ubuntu-latest + outputs: + version: ${{ steps.version.outputs.version }} + + steps: + - name: Checkout code + uses: actions/checkout@v7 + with: + ref: ${{ github.event.release.tag_name }} + + - name: Resolve and verify version + id: version + run: | + TAG="${{ github.event.release.tag_name }}" + VERSION="${TAG#v}" + MANIFEST=$(grep '^version = ' dotscope/Cargo.toml | head -1 | cut -d '"' -f 2) + if [ "$VERSION" != "$MANIFEST" ]; then + echo "::error::Release tag ${TAG} implies version ${VERSION}, but dotscope/Cargo.toml declares ${MANIFEST}" + exit 1 + fi + echo "version=$VERSION" >> $GITHUB_OUTPUT + echo "Releasing v${VERSION}" + + # Then ensure all tests pass test: name: Pre-Release Tests runs-on: ubuntu-latest + needs: version steps: - name: Checkout code uses: actions/checkout@v7 + with: + ref: ${{ github.event.release.tag_name }} - name: Install Rust toolchain uses: dtolnay/rust-toolchain@stable @@ -57,111 +85,11 @@ jobs: - name: Check documentation run: cargo doc --no-deps - # Create GitHub release - create-release: - name: Create Release - runs-on: ubuntu-latest - needs: test - outputs: - version: ${{ steps.version.outputs.version }} - release_created: ${{ steps.check_release.outputs.created }} - - steps: - - name: Checkout code - uses: actions/checkout@v7 - with: - fetch-depth: 0 - - - name: Get version from Cargo.toml - id: version - run: | - VERSION=$(grep '^version = ' dotscope/Cargo.toml | head -1 | cut -d '"' -f 2) - echo "version=$VERSION" >> $GITHUB_OUTPUT - echo "Version: $VERSION" - - - name: Check if release exists - id: check_release - run: | - if gh release view "v${{ steps.version.outputs.version }}" >/dev/null 2>&1; then - echo "created=false" >> $GITHUB_OUTPUT - echo "Release v${{ steps.version.outputs.version }} already exists, skipping" - else - echo "created=true" >> $GITHUB_OUTPUT - fi - env: - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - - - name: Generate changelog - id: changelog - if: steps.check_release.outputs.created == 'true' - run: | - LATEST_TAG=$(git describe --tags --abbrev=0 2>/dev/null || echo "") - - if [ -z "$LATEST_TAG" ]; then - CHANGELOG=$(git log --pretty=format:"- %s (%h)" --no-merges) - else - CHANGELOG=$(git log ${LATEST_TAG}..HEAD --pretty=format:"- %s (%h)" --no-merges) - fi - - if [ -z "$CHANGELOG" ]; then - CHANGELOG="- Initial release" - fi - - echo "CHANGELOG<> $GITHUB_OUTPUT - echo "$CHANGELOG" >> $GITHUB_OUTPUT - echo "EOF" >> $GITHUB_OUTPUT - - - name: Create Release - if: steps.check_release.outputs.created == 'true' - run: | - VERSION="v${{ steps.version.outputs.version }}" - cat < release_body.md - ## Changes in ${VERSION} - - ${{ steps.changelog.outputs.CHANGELOG }} - - ## Installation - - ### Library - - Add this to your \`Cargo.toml\`: - - \`\`\`toml - [dependencies] - dotscope = "${{ steps.version.outputs.version }}" - \`\`\` - - Or install via cargo: - - \`\`\`bash - cargo add dotscope - \`\`\` - - ### CLI Tool - - Download the pre-built binary for your platform from the assets below and extract it. - - | Platform | Asset | - |----------|-------| - | Linux (x86_64) | \`dotscope-${VERSION}-x86_64-unknown-linux-gnu.zip\` | - | macOS (Apple Silicon) | \`dotscope-${VERSION}-aarch64-apple-darwin.zip\` | - | Windows (x86_64) | \`dotscope-${VERSION}-x86_64-pc-windows-msvc.zip\` | - - > **Note:** Z3 is an optional compile-time dependency used only for the \`z3\` feature (CFF reconstruction). The pre-built CLI binaries do **not** require Z3 at runtime. - BODY - - gh release create "$VERSION" \ - --title "Release ${VERSION}" \ - --notes-file release_body.md - env: - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - - # Build CLI binaries for all platforms + # Build CLI binaries for all platforms and attach them to the release build-cli: name: Build CLI (${{ matrix.name }}) runs-on: ${{ matrix.os }} - needs: create-release - if: needs.create-release.outputs.release_created == 'true' + needs: [version, test] strategy: fail-fast: false matrix: @@ -182,6 +110,8 @@ jobs: steps: - name: Checkout code uses: actions/checkout@v7 + with: + ref: ${{ github.event.release.tag_name }} - name: Install Rust toolchain uses: dtolnay/rust-toolchain@stable @@ -200,18 +130,18 @@ jobs: if: runner.os != 'Windows' run: | cd target/${{ matrix.target }}/release - zip dotscope-v${{ needs.create-release.outputs.version }}-${{ matrix.target }}.zip ${{ matrix.artifact }} - mv dotscope-v${{ needs.create-release.outputs.version }}-${{ matrix.target }}.zip ${{ github.workspace }}/ + zip dotscope-v${{ needs.version.outputs.version }}-${{ matrix.target }}.zip ${{ matrix.artifact }} + mv dotscope-v${{ needs.version.outputs.version }}-${{ matrix.target }}.zip ${{ github.workspace }}/ - name: Package binary (Windows) if: runner.os == 'Windows' run: | cd target/${{ matrix.target }}/release - Compress-Archive -Path ${{ matrix.artifact }} -DestinationPath "${{ github.workspace }}\dotscope-v${{ needs.create-release.outputs.version }}-${{ matrix.target }}.zip" + Compress-Archive -Path ${{ matrix.artifact }} -DestinationPath "${{ github.workspace }}\dotscope-v${{ needs.version.outputs.version }}-${{ matrix.target }}.zip" shell: pwsh - name: Upload release asset - run: gh release upload "v${{ needs.create-release.outputs.version }}" "dotscope-v${{ needs.create-release.outputs.version }}-${{ matrix.target }}.zip" --clobber + run: gh release upload "${{ github.event.release.tag_name }}" "dotscope-v${{ needs.version.outputs.version }}-${{ matrix.target }}.zip" --clobber env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} @@ -219,12 +149,13 @@ jobs: publish: name: Publish to crates.io runs-on: ubuntu-latest - needs: [test, create-release] - if: needs.create-release.outputs.release_created == 'true' + needs: [version, test] steps: - name: Checkout code uses: actions/checkout@v7 + with: + ref: ${{ github.event.release.tag_name }} - name: Install Rust toolchain uses: dtolnay/rust-toolchain@stable @@ -244,12 +175,13 @@ jobs: verify-docs: name: Verify Documentation runs-on: ubuntu-latest - needs: [test, create-release] - if: needs.create-release.outputs.release_created == 'true' + needs: [version, test] steps: - name: Checkout code uses: actions/checkout@v7 + with: + ref: ${{ github.event.release.tag_name }} - name: Install Rust toolchain uses: dtolnay/rust-toolchain@stable diff --git a/CHANGELOG.md b/CHANGELOG.md index 80f2cd01..333d2cbe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **`optimize_locals` split values it renumbered** (`SsaFunctionCilExt`): renumbering a local moved its origin but left its rename group pointing at the old slot. `rebuild_ssa` groups variables by rename group while resolving argument/local representatives by origin, so the two views disagreed about which names denote the same local and the value ended up with no reaching definition. Both now move together - **CFF tracer could escape its visit budget**: entering an expression-switch false arm deliberately resets `total_visits` so each arm gets its own budget. On heavily nested methods that reset fired often enough to make the budget unbounded — a 348-block ConfuserEx method reached 3.3 million block visits against a 50,000 cap. A monotonic counter now bounds total work per trace without changing the per-arm semantics - **CFF reconstruction produced invalid SSA after block cloning**: cloned blocks duplicated every definition they contained, phi operands survived pointing at predecessors the patched CFG no longer has, and operands were left naming redirected blocks. Clones now allocate fresh variable ids (after state-tainted filtering, which keys on the original ids), phi operands are pruned against the patched predecessor sets and definedness, and redirected operands are resolved through a bounded hop limit +- **Cleanup deleted live types reachable only through another deletion candidate** (#249, fix contributed in #248 by [@agski331](https://github.com/agski331)): `find_unreferenced_types` computed a single step of what is a transitive reachability problem. A candidate rescued partway through a pass never propagated that liveness onwards, so any cluster reachable solely through it was still read as isolated infrastructure — on `reactor_virtualization` that cut the assembly from 854 methods to 45, deleting a VM interpreter whose stubs remain live because nothing devirtualizes it. Reachability is now a worklist drain over a type-level call graph, in `O(V+E)` rather than `O(depth × edges)`. CustomAttribute constructor types are seeded as roots before propagation rather than filtered afterwards, so the types those constructors call survive too, and liveness propagates from a nested type to its enclosing type — deleting an enclosing type cascades to its children through `expand_type_tokens`, which was dropping nested types that live code still called. The deletion set is sorted for a reproducible order +- **Technique cleanup ran whether or not the transformation it implies succeeded**: a technique builds its cleanup request from detection findings alone, so it scheduled the decryptor, its infrastructure type, the initializer and the encrypted data for deletion even when no call site was reversed, leaving those call sites pointing at metadata that no longer exists and emptying the methods holding them. Removal now requires that nothing still calls the decryptor — a successful decryption rewrites its call site to the constant, so the absence of remaining callers is the evidence of reversal. Absence of recorded failures is not, since a decryptor that was never exercised has none either. Note that the caller check reads the SSA call graph, which covers only methods that converted successfully +- **ConfuserEx constant decryption missed builds that differ from stock 1.6.0** (#249): four defects, each masking the next. The blob index was matched against `int32` alone, so builds emitting `T Get(uint32)` yielded no decryptor at all — any integer width is now accepted, the surrounding constraints carrying the selectivity. `stobj` rejected reference types, though ECMA-335 §III.4.29 defines it over any `typeTok` and makes it equivalent to `stind.ref` for reference types, which is what `stobj !!T` becomes at `T = string`. `ldelema` hand-rolled its index match to `I32`/`NativeInt` while `ldelem`/`stelem` share a helper accepting every width, so a `native uint` index aborted emulation. And `stfld` through a pointer to a value-type array element replaced the whole element with the field's value instead of updating the field inside it, which is the path LZMA's bit-decoder struct arrays take +- **ConfuserEx LZMA blobs were rejected or misparsed** (#249): the sniffer required the compressed payload to be smaller than its declared output, but LZMA expands small high-entropy input and the constants blob is XOR-encrypted before compression — a 44-byte blob compresses to 51 and was refused. The header was also modelled as 5 property bytes plus a 4-byte size, while builds calling the LZMA SDK's stream API write the standard 13-byte header with an 8-byte size; reading that as 9 bytes shifts the payload and corrupts the range coder. Both layouts are now attempted and held to the size each declares, and the size ceiling is documented as an allocation guard rather than a property of the format ### Changed diff --git a/dotscope/src/cilassembly/cleanup/analysis.rs b/dotscope/src/cilassembly/cleanup/analysis.rs index 8a2f1b73..3c567aca 100644 --- a/dotscope/src/cilassembly/cleanup/analysis.rs +++ b/dotscope/src/cilassembly/cleanup/analysis.rs @@ -79,8 +79,12 @@ pub fn expand_type_tokens(request: &CleanupRequest, assembly: &CilObject) -> Has /// in the provided method call graph. /// /// These are self-contained infrastructure types that can be safely deleted. -/// A type is considered unreferenced when no method outside the type (and -/// not already scheduled for deletion) calls any of its methods. +/// Reachability is computed transitively: candidate types are rooted at every +/// live non-candidate type, and a candidate reached from a root is itself +/// promoted to a root. A type is considered unreferenced only when no chain of +/// calls from live code arrives at it. Stopping after a single step would +/// delete clusters that are reachable solely through another candidate — a VM +/// dispatcher or decryptor facade that turns out to be live. /// /// This handles two key scenarios: /// - **Proxy devirtualization**: wrapper types whose methods are @@ -175,9 +179,11 @@ pub fn find_unreferenced_types( } } - // Compute which candidates have live external callers - let mut has_external_caller: HashSet = HashSet::new(); - + // Collapse the method call graph to type granularity, keeping only edges + // whose caller is still live. Doing this once lets reachability be answered + // with a single worklist drain instead of re-scanning every call edge per + // round. + let mut type_calls: HashMap> = HashMap::new(); for (caller_token, callees) in method_call_graph { // Skip callers that are deleted if deleted_methods.contains(caller_token) { @@ -190,35 +196,67 @@ pub fn find_unreferenced_types( if deleted_types.contains(&caller_type) { continue; } - // Skip callers that are themselves candidates — their edges are - // intra-cluster and don't constitute external references - let caller_is_candidate = candidates.contains(&caller_type); for callee_token in callees { let Some(callee_type) = method_to_type.get(callee_token).copied() else { continue; }; + // Intra-type calls say nothing about whether the type is reachable if caller_type == callee_type { continue; } - // Only mark as externally referenced if the caller is NOT a candidate - if !caller_is_candidate && candidates.contains(&callee_type) { - has_external_caller.insert(callee_type); + type_calls + .entry(caller_type) + .or_default() + .insert(callee_type); + } + } + + // A nested type cannot outlive its enclosing type: deleting an enclosing + // type cascades to its nested types via [`expand_type_tokens`]. Model that + // as an edge from nested to enclosing so a live nested type keeps its + // parent alive, rather than being silently dropped along with it. + for type_entry in registry.iter() { + let type_token = *type_entry.key(); + if type_token.table() != 0x02 { + continue; + } + // A type already scheduled for deletion is not live and must never + // become a reachability root + if deleted_types.contains(&type_token) { + continue; + } + if let Some(enclosing) = type_entry.value().enclosing_type() { + if enclosing.token != type_token { + type_calls + .entry(type_token) + .or_default() + .insert(enclosing.token); } } } + // Seed the roots: every live type that is not itself a candidate. Candidacy + // is what reachability has to resolve, so a candidate cannot serve as proof + // that another candidate is referenced. + let mut worklist: Vec = type_calls + .keys() + .filter(|type_token| !candidates.contains(type_token)) + .copied() + .collect(); + // CustomAttribute constructors reference types that the call graph misses. // Types like EmbeddedAttribute and RefSafetyRulesAttribute are only referenced // from CustomAttribute rows (their .ctors are attribute constructors), never // from IL code. Without this check they appear unreferenced and get deleted. + // They join the roots so that whatever they call stays alive as well. if let Some(tables) = assembly.tables() { if let Some(attr_table) = tables.table::() { for row in attr_table { if row.constructor.token.is_table(TableId::MethodDef) { if let Some(&ctor_type) = method_to_type.get(&row.constructor.token) { - if candidates.contains(&ctor_type) { - has_external_caller.insert(ctor_type); + if candidates.remove(&ctor_type) { + worklist.push(ctor_type); } } } @@ -226,11 +264,24 @@ pub fn find_unreferenced_types( } } - // All candidates without external callers are unreferenced infrastructure - candidates - .into_iter() - .filter(|t| !has_external_caller.contains(t)) - .collect() + // Propagate liveness transitively. A candidate rescued here becomes a root + // in turn — without that, a cluster reachable only through another candidate + // (a VM dispatcher, a decryptor facade) is misread as isolated infrastructure. + while let Some(live_type) = worklist.pop() { + let Some(callees) = type_calls.get(&live_type) else { + continue; + }; + for callee_type in callees { + if candidates.remove(callee_type) { + worklist.push(*callee_type); + } + } + } + + // Whatever is still a candidate is unreachable from live code + let mut unreferenced: Vec = candidates.into_iter().collect(); + unreferenced.sort_unstable(); + unreferenced } /// Pre-computes the set of entry-point method tokens that should not be removed. diff --git a/dotscope/src/deobfuscation/cleanup.rs b/dotscope/src/deobfuscation/cleanup.rs index 72add578..ead26594 100644 --- a/dotscope/src/deobfuscation/cleanup.rs +++ b/dotscope/src/deobfuscation/cleanup.rs @@ -55,6 +55,32 @@ use crate::{ CilObject, Result, }; +/// Returns the registered decryptors that still have call sites in the +/// post-pass call graph. +/// +/// Decryption rewrites a call site into the constant it produced, so a +/// decryptor nothing calls any more has been fully reversed and its +/// infrastructure can go. One that is still called has not been reversed — +/// whether because emulation failed, or because the call site was never +/// reached — and neither it nor the type that owns it may be deleted. +/// +/// Generic decryptors are invoked through `MethodSpec` tokens rather than the +/// `MethodDef` directly, so each callee is resolved back to its base decryptor. +fn decryptors_still_called( + ctx: &AnalysisContext, + ssa_call_graph: &BTreeMap>, +) -> HashSet { + let mut still_called = HashSet::new(); + for callees in ssa_call_graph.values() { + for callee in callees { + if let Some(decryptor) = ctx.decryptors.resolve_decryptor(*callee) { + still_called.insert(decryptor); + } + } + } + still_called +} + /// Builds a complete cleanup request from detection results and analysis state. /// /// This consolidates all cleanup sources into a single request: @@ -85,6 +111,7 @@ pub(crate) fn build_cleanup_request( // Start with technique-merged cleanup let registry = engine.technique_registry(); let mut request = detections.merged_cleanup(); + let still_called = decryptors_still_called(ctx, ssa_call_graph); for tech in registry.sorted_techniques(detections) { if !detections.is_detected(tech.id()) { continue; @@ -93,27 +120,47 @@ pub(crate) fn build_cleanup_request( continue; }; if let Some(tech_cleanup) = tech.cleanup(detection) { - request.merge(&tech_cleanup); + // A technique builds its cleanup request from detection findings + // alone — it cannot see whether the transformation it implies + // actually succeeded. Merging it unconditionally deletes the + // decryptor, its owning infrastructure type and the encrypted data + // even when not a single call site was reversed, leaving live call + // sites pointing at methods that no longer exist. Withhold the whole + // request when a decryptor it removes is still called: deleting what + // was never replaced is strictly worse than leaving the obfuscation + // in place. + let unreversed: Vec = tech_cleanup + .methods() + .filter(|t| still_called.contains(t)) + .copied() + .collect(); + if unreversed.is_empty() { + request.merge(&tech_cleanup); + } else { + log::warn!( + "Skipping cleanup for {}: {} decryptor(s) still have live call sites \ + ({} decryption failures); their infrastructure must survive", + tech.id(), + unreversed.len(), + ctx.decryptors.total_failed(), + ); + } } } - // Protect registered decryptors that were NOT fully decrypted. - // Techniques may unconditionally mark decryptor methods for cleanup, but - // if emulation failed for any call site, the method must survive — deleting - // it would leave broken call sites that reference a now-missing method. - // Only fully-decrypted methods are safe to remove. - let removable = ctx.decryptors.removable_decryptors(); + // Protect decryptors that anything still calls, and remove the rest. + // A successful decryption rewrites its call site to the decrypted constant, + // so a decryptor with no remaining callers has been fully reversed and is + // safe to delete. One that is still called has not been — deleting it would + // leave those call sites referencing a method that no longer exists. for token in ctx.decryptors.registered_tokens() { - if !removable.contains(&token) { + if still_called.contains(&token) { request.protect_token(token); + } else { + request.add_method(token); } } - // Add decryptors that were fully emulated and are now safe to remove. - for token in removable { - request.add_method(token); - } - // Pre-scan: mark methods as dead if they have no live callers. sweep_dead_module_methods(assembly, &mut request, ssa_call_graph, ctx); diff --git a/dotscope/src/deobfuscation/decryptors.rs b/dotscope/src/deobfuscation/decryptors.rs index 4eba405e..80676d43 100644 --- a/dotscope/src/deobfuscation/decryptors.rs +++ b/dotscope/src/deobfuscation/decryptors.rs @@ -544,8 +544,12 @@ impl DecryptorContext { /// Checks if all calls to a decryptor were successfully handled. /// - /// Returns `true` if there are no failed calls for this decryptor, - /// meaning it's safe to remove. + /// Returns `true` if there are no failed calls for this decryptor. + /// + /// Note that this is not on its own sufficient to justify deleting the + /// method: a decryptor that was never exercised also has no failures. + /// Removal additionally requires that nothing still calls it — see + /// `decryptors_still_called` in the cleanup builder. /// /// # Arguments /// @@ -553,7 +557,7 @@ impl DecryptorContext { /// /// # Returns /// - /// `true` if safe to remove (no failed calls). + /// `true` if no call site failed to decrypt. #[must_use] pub fn is_fully_decrypted(&self, decryptor: Token) -> bool { self.failed.get(&decryptor).is_none_or(|r| r.count() == 0) diff --git a/dotscope/src/deobfuscation/techniques/confuserex/constants.rs b/dotscope/src/deobfuscation/techniques/confuserex/constants.rs index c150fcad..014e4ade 100644 --- a/dotscope/src/deobfuscation/techniques/confuserex/constants.rs +++ b/dotscope/src/deobfuscation/techniques/confuserex/constants.rs @@ -264,23 +264,40 @@ impl Technique for ConfuserExConstants { let sig = &method.signature; - // Check for string(int32) signature - let first_param_is_i4 = sig - .params - .first() - .is_some_and(|p| p.base == TypeSignature::I4); + // The single parameter is an index into the constant blob. Its + // declared width varies across ConfuserEx forks — stock 1.6.0 + // emits `int32`, others emit `uint32` — so accept any integral + // type rather than pinning one. The surrounding constraints + // (static, or non-ASCII owner, arity 1, string or `!!0` + // return) carry the selectivity. + let first_param_is_integral = sig.params.first().is_some_and(|p| { + matches!( + p.base, + TypeSignature::I1 + | TypeSignature::U1 + | TypeSignature::I2 + | TypeSignature::U2 + | TypeSignature::I4 + | TypeSignature::U4 + | TypeSignature::I8 + | TypeSignature::U8 + | TypeSignature::I + | TypeSignature::U + ) + }); + // Check for string() signature let is_string_decryptor = sig.param_count_generic == 0 && sig.return_type.base == TypeSignature::String && sig.params.len() == 1 - && first_param_is_i4; + && first_param_is_integral; - // Check for generic T(int32) signature (param_count_generic == 1, + // Check for generic T() signature (param_count_generic == 1, // return type is GenericParamMethod(0)) let is_generic_decryptor = sig.param_count_generic == 1 && matches!(sig.return_type.base, TypeSignature::GenericParamMethod(0)) && sig.params.len() == 1 - && first_param_is_i4; + && first_param_is_integral; if is_string_decryptor || is_generic_decryptor { decryptor_tokens.push(method.token); diff --git a/dotscope/src/emulation/engine/interpreter/handlers.rs b/dotscope/src/emulation/engine/interpreter/handlers.rs index ae5f8291..0406deba 100644 --- a/dotscope/src/emulation/engine/interpreter/handlers.rs +++ b/dotscope/src/emulation/engine/interpreter/handlers.rs @@ -943,18 +943,11 @@ impl Interpreter { let index = thread.pop()?; let array_ref = thread.pop()?; - let idx_i64 = match index { - EmValue::I32(v) => i64::from(v), - EmValue::NativeInt(v) => v, - _ => { - return Err(EmulationError::TypeMismatch { - operation: "ldelema", - expected: "integer index", - found: index.cil_flavor().as_str(), - } - .into()) - } - }; + // Share the index extraction used by `ldelem`/`stelem` rather than + // matching a narrower set here. `native uint` is a legal array index and + // appears in ConfuserEx's constants `Initialize()`; accepting only + // `I32`/`NativeInt` rejected it and aborted the whole emulation. + let idx_i64 = Self::extract_array_index(thread, &index, "ldelema")?; let idx = usize::try_from(idx_i64).map_err(|_| { Error::from(EmulationError::ArrayIndexOutOfBounds { @@ -1372,6 +1365,39 @@ impl Interpreter { } } } + PointerTarget::ArrayElement { array, index } => { + // `ldelema` on a value-type array yields a pointer to the + // element itself, and `stfld` through it must update one + // field *within* that element. Storing through the pointer + // would replace the whole element with the field value, so + // read-modify-write the struct instead. LZMA's bit-decoder + // arrays in ConfuserEx's constants runtime take this path. + let element = thread.heap().get_array_element(*array, *index).ok(); + match element { + Some(vt @ EmValue::ValueType { .. }) => { + if let Some(updated) = + store_into_valuetype(thread, vt, field_token, value.clone()) + { + thread + .heap_mut() + .set_array_element(*array, *index, updated)?; + return Ok(StepResult::Continue); + } + thread.store_through_pointer(&ptr, value)?; + Ok(StepResult::Continue) + } + Some(EmValue::ObjectRef(href)) => { + // Reference-type element: the field belongs to the + // referenced object, not to the array slot. + thread.heap_mut().set_field(href, field_token, value)?; + Ok(StepResult::Continue) + } + _ => { + thread.store_through_pointer(&ptr, value)?; + Ok(StepResult::Continue) + } + } + } _ => { // For other pointer targets, store through the pointer thread.store_through_pointer(&ptr, value)?; @@ -1964,19 +1990,21 @@ impl Interpreter { let value = thread.pop()?; let addr = thread.pop()?; - // Verify this is a value type operation (stobj is for value types) + // `stobj` is not restricted to value types. ECMA-335 III.4.29 defines it + // over any `typeTok`, and states that when `typeTok` is a reference type + // the instruction is equivalent to `stind.ref`. Generic code relies on + // this: a method with a `T&` destination emits `stobj !!T`, which stores + // a reference whenever `T` is instantiated with a reference type — the + // shape ConfuserEx's `T Get(uint)` decryptor takes at `T = string`. + // + // `type_token` is not resolved here, so there is nothing meaningful to + // validate the value against; only `Void` is categorically unstorable. let value_type = value.cil_flavor(); - let is_value_type = value_type == CilFlavor::ValueType - || value_type == CilFlavor::I4 - || value_type == CilFlavor::I8 - || value_type == CilFlavor::R4 - || value_type == CilFlavor::R8 - || value_type == CilFlavor::I; - if !is_value_type { + if value_type == CilFlavor::Void { let _ = type_token; // Token available for future type resolution return Err(EmulationError::TypeMismatch { operation: "stobj", - expected: "value type", + expected: "storable value", found: value_type.as_str(), } .into()); diff --git a/dotscope/src/utils/decompress.rs b/dotscope/src/utils/decompress.rs index b348da5f..a967c44c 100644 --- a/dotscope/src/utils/decompress.rs +++ b/dotscope/src/utils/decompress.rs @@ -6,10 +6,17 @@ //! //! # ConfuserEx LZMA Format //! -//! ConfuserEx uses a custom LZMA format: -//! - 5 bytes: LZMA decoder properties -//! - 4 bytes: Uncompressed size (little-endian i32) -//! - Rest: Compressed data stream +//! Two header layouts appear in the wild, sharing the same 5 property bytes and +//! differing only in the width of the uncompressed-size field that follows: +//! +//! - 5 bytes properties + 8 bytes size (little-endian u64) — the standard +//! `.lzma` (alone) header, written by builds that drive the LZMA SDK's stream +//! API directly +//! - 5 bytes properties + 4 bytes size (little-endian u32) — stock ConfuserEx's +//! `Lzma.Decompress` +//! +//! Nothing in the assembly records which is in use, so both are attempted and +//! each is held to the length its own header declares. //! //! # Deflate Format //! @@ -48,30 +55,33 @@ impl std::fmt::Display for DecompressError { impl std::error::Error for DecompressError {} -/// Checks if the given data appears to be ConfuserEx LZMA format. +/// Upper bound on a declared decompressed size, in bytes. /// -/// ConfuserEx LZMA format: -/// - 5 bytes: LZMA properties (first byte typically 0x5D for default settings) -/// - 4 bytes: Uncompressed size (little-endian i32, may be negative for unknown) -/// - Rest: Compressed LZMA stream +/// This is a guard against corrupt or hostile length fields driving a large +/// allocation, **not** a property of the format — LZMA itself allows any +/// `u64`. The bound only has to sit far above anything a real assembly +/// produces: a constants blob holds an assembly's literals, which run to +/// kilobytes for typical programs and single-digit megabytes for pathological +/// ones, so 512 MB leaves several orders of magnitude of headroom while still +/// rejecting a field that is plainly nonsense. +const MAX_DECOMPRESSED_SIZE: u64 = 512 * 1024 * 1024; + +/// Header layouts used by ConfuserEx and its forks. /// -/// # Arguments +/// Both start with the same 5 property bytes and differ only in the width of +/// the uncompressed-size field that follows. +const LZMA_HEADER_LAYOUTS: [usize; 2] = [ + 13, // 5 props + 8-byte size — the standard `.lzma` (alone) header + 9, // 5 props + 4-byte size — stock ConfuserEx's `Lzma.Decompress` +]; + +/// Validates the 5 LZMA property bytes shared by every supported layout. /// -/// * `data` - The potentially compressed data. -/// -/// # Returns -/// -/// `true` if the data appears to be ConfuserEx LZMA format. -#[must_use] -pub fn is_confuserex_lzma(data: &[u8]) -> bool { - if data.len() < 13 { - // Need at least 9 bytes header + some compressed data - return false; - } - - // LZMA properties byte: encodes lc, lp, pb parameters - // Valid range: 0-224 (9 * 5 * 5 - 1) - // ConfuserEx typically uses default settings: lc=3, lp=0, pb=2 -> 0x5D +/// Checks the properties byte encodes a legal `lc`/`lp`/`pb` triple and that +/// the dictionary size is one a real encoder would choose. +fn valid_lzma_props(data: &[u8]) -> bool { + // Properties byte encodes (pb * 5 + lp) * 9 + lc; the largest legal value + // is 9 * 5 * 5 - 1 = 224. let Some(&props_byte) = data.first() else { return false; }; @@ -79,46 +89,72 @@ pub fn is_confuserex_lzma(data: &[u8]) -> bool { return false; } - // For better heuristics, check if props byte matches common LZMA settings - // 0x5D is the most common (lc=3, lp=0, pb=2) - // But we allow any valid props for flexibility - - // Check dictionary size (bytes 1-4 of LZMA properties) - // ConfuserEx typically uses 1MB dictionary (0x00100000) + // Dictionary size: the SDK's presets span 64 KB to 64 MB depending on the + // fork's compression level, so accept a generous band around that. let Some(dict_bytes) = data.get(1..5).and_then(|s| <[u8; 4]>::try_from(s).ok()) else { return false; }; let dict_size = u32::from_le_bytes(dict_bytes); + (1024u32..=1024u32.saturating_mul(1024).saturating_mul(1024)).contains(&dict_size) +} - // Dictionary size must be reasonable: 1KB to 16MB - // Too small or too large suggests this isn't LZMA - if !(1024u32..=16u32.saturating_mul(1024).saturating_mul(1024)).contains(&dict_size) { - return false; - } - - // Bytes 5-8: Uncompressed size (little-endian i32) - // For ConfuserEx, this is typically a small positive number (the decrypted constants) - let Some(size_bytes) = data.get(5..9).and_then(|s| <[u8; 4]>::try_from(s).ok()) else { - return false; +/// Reads the declared uncompressed size for a given header length. +/// +/// Returns `None` when the buffer is too short, or the size is zero or beyond +/// [`MAX_DECOMPRESSED_SIZE`]. An all-ones field means "unknown" and maps to +/// `Some(None)` — plausible, but with no length to verify against. +fn declared_size(data: &[u8], header_len: usize) -> Option> { + let size = match header_len { + 13 => u64::from_le_bytes(data.get(5..13)?.try_into().ok()?), + _ => u64::from(u32::from_le_bytes(data.get(5..9)?.try_into().ok()?)), }; - let uncompressed_size = i32::from_le_bytes(size_bytes); + // `-1` in either width signals an unknown length + if size == u64::MAX || size == u64::from(u32::MAX) { + return Some(None); + } + if size == 0 || size > MAX_DECOMPRESSED_SIZE { + return None; + } + Some(Some(size)) +} - // Uncompressed size should be positive and reasonable (< 10MB) - // Negative or very large sizes indicate this isn't LZMA data - if uncompressed_size <= 0 || uncompressed_size > 10i32.saturating_mul(1024).saturating_mul(1024) - { +/// Checks if the given data appears to be ConfuserEx LZMA format. +/// +/// Two header layouts are recognised, both sharing the same 5 property bytes +/// and differing in the width of the uncompressed-size field: +/// +/// - 5 bytes properties + 8 bytes size — the standard `.lzma` (alone) header, +/// used by forks that call the LZMA SDK's stream API directly. +/// - 5 bytes properties + 4 bytes size — stock ConfuserEx's `Lzma.Decompress`. +/// +/// # Arguments +/// +/// * `data` - The potentially compressed data. +/// +/// # Returns +/// +/// `true` if the data appears to be ConfuserEx LZMA format. +#[must_use] +pub fn is_confuserex_lzma(data: &[u8]) -> bool { + if data.len() < 13 { + // Need at least the largest header plus some compressed data return false; } - // Additional check: compressed data should be smaller than uncompressed - // (otherwise why compress it?) - let compressed_data_len = data.len().saturating_sub(9); // minus header - if compressed_data_len > uncompressed_size.cast_unsigned() as usize { - // Compressed larger than uncompressed - suspicious + if !valid_lzma_props(data) { return false; } - true + // Accept if either header layout yields a plausible declared size. + // + // Deliberately *not* checked: that the compressed payload is smaller than + // the decompressed output. LZMA expands small high-entropy inputs, and the + // constants blob is XOR-encrypted before compression — so a 44-byte blob + // routinely compresses to more than 44 bytes. Requiring shrinkage rejected + // exactly the assemblies this hook exists to handle. + LZMA_HEADER_LAYOUTS + .iter() + .any(|&len| declared_size(data, len).is_some()) } /// Decompresses ConfuserEx LZMA data. @@ -133,47 +169,67 @@ pub fn is_confuserex_lzma(data: &[u8]) -> bool { /// /// # Format /// +/// Two layouts are attempted, sharing the same 5 property bytes and differing +/// in the width of the uncompressed-size field: +/// /// ```text /// [0..5] : LZMA properties (5 bytes) -/// [5..9] : Uncompressed size (4 bytes, little-endian i32) +/// [5..13] : Uncompressed size (8 bytes, little-endian u64) -- standard `.lzma` +/// [13..] : LZMA compressed stream +/// +/// [0..5] : LZMA properties (5 bytes) +/// [5..9] : Uncompressed size (4 bytes, little-endian u32) -- stock ConfuserEx /// [9..] : LZMA compressed stream /// ``` +/// +/// The layout is not recorded anywhere in the assembly, so both are tried and +/// the first that decodes to the length its own header declares wins. Guessing +/// wrong shifts the payload by four bytes and corrupts the range coder, so a +/// successful decode is itself the discriminator. pub fn decompress_confuserex_lzma(data: &[u8]) -> DecompressResult> { if data.len() < 9 { return Err(DecompressError::BufferTooSmall); } + if !valid_lzma_props(data) { + return Err(DecompressError::InvalidLzmaHeader); + } - // Parse header let props = data.get(0..5).ok_or(DecompressError::BufferTooSmall)?; - let size_bytes = data - .get(5..9) - .and_then(|s| <[u8; 4]>::try_from(s).ok()) - .ok_or(DecompressError::BufferTooSmall)?; - let uncompressed_size = i32::from_le_bytes(size_bytes); - let compressed = data.get(9..).ok_or(DecompressError::BufferTooSmall)?; - - // Build LZMA stream header for lzma-rs - // lzma-rs expects: 5 bytes props + 8 bytes uncompressed size (little-endian u64) + compressed data - let mut lzma_stream = Vec::with_capacity(compressed.len().saturating_add(13)); - lzma_stream.extend_from_slice(props); - - // Convert i32 to u64 for lzma-rs format - let size_u64 = if uncompressed_size < 0 { - u64::MAX // Unknown size - } else { - u64::from(uncompressed_size.cast_unsigned()) - }; - lzma_stream.extend_from_slice(&size_u64.to_le_bytes()); - lzma_stream.extend_from_slice(compressed); - - // Decompress using lzma-rs - let mut cursor = Cursor::new(&lzma_stream); - let mut decompressed = Vec::new(); + let mut last_err: Option = None; + + for &header_len in &LZMA_HEADER_LAYOUTS { + let Some(size) = declared_size(data, header_len) else { + continue; + }; + let Some(compressed) = data.get(header_len..) else { + continue; + }; + if compressed.is_empty() { + continue; + } - lzma_rs::lzma_decompress(&mut cursor, &mut decompressed) - .map_err(|e| DecompressError::LzmaError(e.to_string()))?; + // lzma-rs expects the alone format: 5 props + 8-byte size + payload. + let mut lzma_stream = Vec::with_capacity(compressed.len().saturating_add(13)); + lzma_stream.extend_from_slice(props); + lzma_stream.extend_from_slice(&size.unwrap_or(u64::MAX).to_le_bytes()); + lzma_stream.extend_from_slice(compressed); + + let mut cursor = Cursor::new(&lzma_stream); + let mut decompressed = Vec::new(); + match lzma_rs::lzma_decompress(&mut cursor, &mut decompressed) { + Ok(()) => { + // A wrong layout can still decode into garbage of the wrong + // length; hold the result to its declared size when known. + if size.is_none_or(|expected| decompressed.len() as u64 == expected) { + return Ok(decompressed); + } + last_err = Some(DecompressError::InvalidLzmaHeader); + } + Err(e) => last_err = Some(DecompressError::LzmaError(e.to_string())), + } + } - Ok(decompressed) + Err(last_err.unwrap_or(DecompressError::InvalidLzmaHeader)) } /// Decompresses Deflate data using flate2. @@ -258,6 +314,71 @@ mod tests { assert!(!is_confuserex_lzma(&too_small)); } + /// A real ConfuserEx constants blob, taken from an assembly produced by a + /// fork that writes the standard 13-byte `.lzma` header (5 property bytes + /// followed by an 8-byte size) rather than stock ConfuserEx's 9-byte one. + /// + /// 51 bytes of payload decode to 44 bytes of output — the compressed form + /// is *larger* than the decompressed form, because the constants blob is + /// XOR-encrypted before compression and LZMA cannot shrink high-entropy + /// input that small. + const FORK_BLOB: [u8; 64] = [ + 0x5D, 0x00, 0x00, 0x80, 0x00, // props: lc=3 lp=0 pb=2, 8 MB dictionary + 0x2C, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // size: 44, as u64 + 0x00, 0x04, 0x00, 0x35, 0x03, 0xA1, 0xBC, 0x67, 0x7D, 0x8E, 0xD0, 0x35, 0x60, 0x52, 0x59, + 0x6E, 0x4A, 0xF6, 0x76, 0x12, 0xF7, 0xD1, 0x80, 0xD2, 0xA5, 0xEA, 0x78, 0xC3, 0x73, 0x0E, + 0x4B, 0x7C, 0xD8, 0x8E, 0xF4, 0xE6, 0x1C, 0x93, 0x81, 0x81, 0x68, 0xCC, 0xEC, 0x3A, 0x04, + 0x8E, 0x00, 0x00, 0x00, 0x00, 0x00, + ]; + + #[test] + fn test_confuserex_lzma_standard_13_byte_header() { + assert!(is_confuserex_lzma(&FORK_BLOB)); + + let out = decompress_confuserex_lzma(&FORK_BLOB).unwrap(); + assert_eq!(out.len(), 44, "must honour the declared size"); + // Length-prefixed UTF-8 constants, as ConfuserEx stores them. + assert!(out.windows(8).any(|w| w == b"Result: ")); + assert!(out.windows(27).any(|w| w == b"Hello From ConfuserEx test.")); + } + + #[test] + fn test_confuserex_lzma_accepts_payload_larger_than_output() { + // Guards a heuristic that used to reject any blob whose payload was + // bigger than its declared output, on the reasoning that compression + // shrinks data. It does not, for small high-entropy inputs — and that + // rejected precisely the assemblies this path exists to decompress. + assert!(FORK_BLOB.len() - 13 > 44); + assert!(is_confuserex_lzma(&FORK_BLOB)); + } + + #[test] + fn test_confuserex_lzma_stock_9_byte_header() { + // Stock ConfuserEx writes the size as 4 bytes, putting the payload at + // offset 9. Rebuild that layout from an alone-format stream. + let original = b"ConfuserEx constants blob, repeated repeated repeated repeated."; + let mut alone = Vec::new(); + lzma_rs::lzma_compress(&mut Cursor::new(&original[..]), &mut alone).unwrap(); + + let mut stock = Vec::with_capacity(alone.len()); + stock.extend_from_slice(&alone[0..5]); + stock.extend_from_slice(&(original.len() as u32).to_le_bytes()); + stock.extend_from_slice(&alone[13..]); + + assert!(is_confuserex_lzma(&stock)); + assert_eq!(decompress_confuserex_lzma(&stock).unwrap(), original); + } + + #[test] + fn test_confuserex_lzma_rejects_non_lzma() { + // Plausible length, but the dictionary size is nonsense. + let junk = [ + 0x5D, 0x11, 0x22, 0x33, 0x44, 0x01, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, + ]; + assert!(!is_confuserex_lzma(&junk)); + assert!(decompress_confuserex_lzma(&junk).is_err()); + } + #[test] fn test_decompress_deflate() { let original = b"Hello, World! This is a test of deflate compression."; diff --git a/dotscope/tests/netreactor.rs b/dotscope/tests/netreactor.rs index 55c0dbe3..e830c15d 100644 --- a/dotscope/tests/netreactor.rs +++ b/dotscope/tests/netreactor.rs @@ -459,9 +459,26 @@ fn test_all_netreactor_samples() { continue; } + // Over-cleanup guard. Virtualized samples are not devirtualized by any + // technique, so the VM interpreter and its handler types remain live + // code — the stubs left in the virtualized methods still call into + // them. Cleanup reachability analysis must follow candidate-to-candidate + // call edges transitively to see that; a single-step version reads the + // handler cluster as isolated infrastructure and strips the assembly + // down to its application methods (854 -> 45 on reactor_virtualization). + if result.success && result.sample.expected_protections.has_virtualization { + assert!( + result.methods_after * 2 > result.methods_before, + "{}: cleanup removed {} of {} methods — the VM runtime is still \ + referenced by the virtualized method stubs and must survive", + result.sample.filename, + result.methods_before.saturating_sub(result.methods_after), + result.methods_before + ); + } + // Once detection is implemented, add assertions here: // - Verify correct technique detection per sample - // - Verify deobfuscation success // - Verify semantic preservation } }