fixes

Author: marirs

Cargo.toml

@@ -79,7 +79,7 @@ iced-x86 = { version = "1", default-features = false, features = ["std", "decode
 # Three direct deps (`miniz_oxide`, `smallvec`, `thiserror`); ~240×
 # faster on `matches()` and ~70% less resident memory than the
 # upstream-FLIRT engine we briefly piloted on.
-fast-flirt = "0.2.1"
+fast-flirt = "0.2.2"
 # 0.4.2: rayon for inter-function parallelism in `find_capabilities`.
 # Each function's analysis is pure — reads the extractor, evaluates
 # rules, returns matches. Parallelising the outer loop gives ~4-8×

src/extractor/dnfile.rs

@@ -22,12 +22,34 @@ struct Instruction {
 }
 
 impl super::Instruction for Instruction {
+    /// CIL is a stack-based ISA — there is no notion of a "stack
+    /// variable" in the x86 sense. Locals are addressed by index
+    /// (`ldloc.N` / `stloc.N`), not by frame offset, and the
+    /// stack-string heuristic that drives `is_mov_imm_to_stack` on
+    /// x86 backends doesn't apply. Python capa makes the same call —
+    /// it doesn't define this method on the dnfile extractor at all.
+    /// Safe default `false` matches Python's behaviour: the
+    /// stack-string detector skips .NET methods.
+    ///
+    /// 0.4.x: previously `unimplemented!()`. Replaced in 0.5.0 — the
+    /// panic was a latent footgun for any caller that held a
+    /// `&dyn Instruction` without knowing which extractor produced it.
     fn is_mov_imm_to_stack(&self) -> Result<bool> {
-        unimplemented!()
+        Ok(false)
     }
+
+    /// CIL string literals come from the `#US` heap via `ldstr`, not
+    /// as byte arrays moved to a stack frame, so there are no
+    /// "printable bytes" to count on an instruction. Python capa
+    /// doesn't define this for the dnfile extractor. Safe default
+    /// `0` keeps the stack-string aggregator at 0 contributions for
+    /// .NET methods — same outcome as Python.
+    ///
+    /// 0.4.x: previously `unimplemented!()`.
     fn get_printable_len(&self) -> Result<u64> {
-        unimplemented!()
+        Ok(0)
     }
+
     fn as_any(&self) -> &dyn std::any::Any {
         self
     }
@@ -36,17 +58,41 @@ impl super::Instruction for Instruction {
 #[derive(Debug, Clone)]
 struct Function {
     f: cil::function::Function,
+    /// Python capa convention: `calls_to` = set of CALLERS (incoming
+    /// references — addresses of methods that call this one).
     calls_to: HashSet<u64>,
+    /// Python capa convention: `calls_from` = set of CALLEES (outgoing
+    /// references — addresses of methods this one calls).
     calls_from: HashSet<u64>,
+    /// Materialised caller list for the `inrefs` trait method. Same
+    /// content as `calls_to`, kept as `Vec` because the trait returns
+    /// `&Vec<u64>`. Populated at `get_functions` construction.
+    inrefs: Vec<u64>,
+    /// Per-block successor map for the `blockrefs` trait method. The
+    /// .NET extractor treats each method as a single basic block
+    /// (matches `get_blocks` below), so this is always
+    /// `{first_insn_offset: vec![]}` — one entry, no successors. A
+    /// future CIL CFG split (br/brtrue/brfalse/switch walking) would
+    /// expand this; tracked as a follow-up.
+    blockrefs: HashMap<u64, Vec<u64>>,
 }
 
 impl super::Function for Function {
+    /// Callers of this method. 0.4.x: `unimplemented!()`. 0.5.0:
+    /// populated from `calls_to` at construction.
     fn inrefs(&self) -> &Vec<u64> {
-        unimplemented!()
+        &self.inrefs
     }
+
+    /// Per-basic-block successor map. 0.4.x: `unimplemented!()`.
+    /// 0.5.0: returns a single-entry map matching `get_blocks`'
+    /// single-block-per-method shape. Proper CIL CFG split is a
+    /// follow-up — it requires walking br/brtrue/brfalse/switch
+    /// targets and re-segmenting the instruction list.
     fn blockrefs(&self) -> &HashMap<u64, Vec<u64>> {
-        unimplemented!()
+        &self.blockrefs
     }
+
     fn offset(&self) -> u64 {
         self.f.offset as u64
     }
@@ -179,11 +225,27 @@ impl<'data> super::Extractor for Extractor<'data> {
     }
 
     fn get_functions(&self) -> Result<std::collections::BTreeMap<u64, Box<dyn super::Function>>> {
-        let mut methods: std::collections::HashMap<u64, Function> =
-            std::collections::HashMap::new();
-        let mut calls_to_map = HashMap::new();
+        // 0.5.0: rebuilt to match Python capa's `calls_to` (callers,
+        // incoming) / `calls_from` (callees, outgoing) convention.
+        // Pre-0.5.0 had two bugs that compounded into a silently-wrong
+        // call graph for every .NET assembly:
+        //   1. `calls_to` was initialised empty and never populated.
+        //   2. The reverse-index loop overwrote `f.calls_from` with the
+        //      caller set instead of populating `f.calls_to`, which
+        //      both inverted the `calls from` characteristic feature
+        //      and broke the `recursive call` detector (it checked
+        //      `f.calls_to`, which was always empty).
+        //
+        // First pass builds `callers_of[target] = {caller, ...}` and
+        // records each function's outgoing callee set. Second pass
+        // assembles the `Function` values with both fields filled in
+        // the correct direction.
+        let mut callers_of: HashMap<u64, HashSet<u64>> = HashMap::new();
+        let mut callees_of: HashMap<u64, HashSet<u64>> = HashMap::new();
+
         for f in self.pe().net()?.functions() {
-            let mut calls_from = HashSet::new();
+            let caller_addr = f.offset as u64;
+            let mut my_callees: HashSet<u64> = HashSet::new();
             for insn in &f.instructions {
                 if ![
                     OpCodeValue::Call,
@@ -195,25 +257,40 @@ impl<'data> super::Extractor for Extractor<'data> {
                 {
                     continue;
                 }
-                let address = insn.operand.value()?;
-                let ee = calls_to_map.entry(address as u64).or_insert(HashSet::new());
-                ee.insert(f.offset as u64);
-                calls_from.insert(address as u64);
+                let target = insn.operand.value()? as u64;
+                my_callees.insert(target);
+                callers_of.entry(target).or_default().insert(caller_addr);
+            }
+            callees_of.insert(caller_addr, my_callees);
+        }
+
+        let mut methods: HashMap<u64, Function> = HashMap::new();
+        for f in self.pe().net()?.functions() {
+            let addr = f.offset as u64;
+            let calls_to = callers_of.remove(&addr).unwrap_or_default();
+            let calls_from = callees_of.remove(&addr).unwrap_or_default();
+            // `inrefs` is just the materialised caller list (the trait
+            // returns `&Vec<u64>`; `calls_to` is a `HashSet`).
+            let inrefs: Vec<u64> = calls_to.iter().copied().collect();
+            // Single-block-per-method shape matches `get_blocks`.
+            // First instruction's offset is the block key; no
+            // intra-function successors at this granularity.
+            let mut blockrefs: HashMap<u64, Vec<u64>> = HashMap::new();
+            if let Some(first) = f.instructions.first() {
+                blockrefs.insert(first.offset as u64, Vec::new());
             }
             methods.insert(
-                f.offset as u64,
+                addr,
                 Function {
                     f: f.clone(),
-                    calls_to: HashSet::new(),
+                    calls_to,
                     calls_from,
+                    inrefs,
+                    blockrefs,
                 },
             );
         }
-        for (a, calls_from) in calls_to_map.into_iter() {
-            if let Some(f) = methods.get_mut(&a) {
-                f.calls_from = calls_from;
-            }
-        }
+
         Ok(methods
             .into_iter()
             .map(|(a, b)| (a, Box::new(b) as Box<dyn super::Function>))
@@ -654,7 +731,13 @@ impl<'data> Extractor<'data> {
         &self,
         f: &Function,
     ) -> Result<Vec<(crate::rules::features::Feature, u64)>> {
-        Ok(f.calls_to
+        // 0.5.0: read `calls_from` (callees) not `calls_to` (callers).
+        // Pre-0.5.0 this was a copy-paste from
+        // `extract_function_call_to_features` that silently emitted the
+        // caller set for every `characteristic: calls from` rule on
+        // .NET assemblies. Matches Python capa's
+        // `extract_function_calls_from` in dnfile/function.py.
+        Ok(f.calls_from
             .iter()
             .map(|a| {
                 (

src/extractor/smda.rs

@@ -201,6 +201,24 @@ impl<'data> super::Extractor for Extractor<'data> {
         f: &Box<dyn super::Function>,
     ) -> Result<Vec<(crate::rules::features::Feature, u64)>> {
         let mut res = vec![]; //extract function calls to
+
+        // 0.5.0 note: function-scope FunctionName emission was
+        // considered (E2 in the parity audit) for parity with Python
+        // capa, but reverted after empirical measurement showed:
+        //   1. The entire capa-rules corpus contains exactly one
+        //      `function-name:` rule, and it's file-scoped, not
+        //      function-scoped. So function-scope emission unlocks
+        //      zero rule matches today.
+        //   2. The emission added ~16% to mimikatz analysis time
+        //      (~0.9s on a 5s baseline) for zero functional benefit
+        //      on real workloads — every emission was a wasted
+        //      String + HashSet alloc that downstream rule eval had
+        //      to probe past.
+        // File-scope FunctionName via `extract_file_function_names`
+        // covers the one rule that exists. If a future Python-capa
+        // release ships function-scope name rules and they're added
+        // to the corpus, revisit this.
+
         for inref in f.inrefs() {
             res.push((
                 crate::rules::features::Feature::Characteristic(

src/flirt.rs

@@ -37,10 +37,10 @@
 //!   policy. For most MSVC patterns this gives the same answer
 //!   because the head-bytes + CRC are unique enough. Refinement is
 //!   tracked as a follow-up.
-//! - **No .pat.gz support yet** — only `.sig` (binary) and `.pat`
-//!   (ASCII) are accepted. The Mandiant FLARE corpus ships as
-//!   `.sig`, so this isn't a practical limitation for the default
-//!   workflow.
+//! - **`.pat.gz` is supported as of 0.5.0** via fast-flirt 0.2.2's
+//!   `add_pat_gz` (pure-Rust gunzip — gzip framing stripped, deflate
+//!   stream inflated via `miniz_oxide`). Pre-0.5.0 `.pat.gz` files
+//!   were counted and surfaced in the load summary but skipped.
 //!
 //! ## Why a hand-rolled walker rather than `fast_flirt::FlirtSet::load_dir`
 //!
@@ -81,12 +81,10 @@ pub struct FlirtMatcher {
 }
 
 impl FlirtMatcher {
-    /// Load all `.sig` and `.pat` files from a directory tree
-    /// (recursive). Files that fail to parse are reported through
+    /// Load all `.sig`, `.pat`, and `.pat.gz` files from a directory
+    /// tree (recursive). Files that fail to parse are reported through
     /// `logger` and skipped — they don't abort the build, matching
-    /// Python capa's best-effort behaviour. `.pat.gz` files are
-    /// counted and surfaced in the final summary (gzipped pat is a
-    /// 0.4.x limitation — gunzip ahead of time as a workaround).
+    /// Python capa's best-effort behaviour.
     ///
     /// Returns an error if `path` can't be read or contains no valid
     /// signatures. A successful matcher with zero signatures would
@@ -108,7 +106,6 @@ impl FlirtMatcher {
         // that's not what we want here.
         let mut builder = FlirtSetBuilder::new();
         let mut source_count = 0usize;
-        let mut gz_skipped = 0usize;
 
         for entry in walkdir::WalkDir::new(path)
             .follow_links(false)
@@ -149,11 +146,22 @@ impl FlirtMatcher {
                     }
                 }
             } else if lower.ends_with(".pat.gz") {
-                // 0.4.x limitation: gzipped pat files not yet
-                // unpacked. Counted separately so the summary line
-                // can call attention to it.
-                gz_skipped += 1;
-                false
+                // 0.5.0: gzipped .pat unpacked via fast-flirt's
+                // pure-Rust gunzip (miniz_oxide). Pre-0.5.0 these
+                // were silently counted-and-skipped.
+                match std::fs::read(p) {
+                    Ok(bytes) => match builder.add_pat_gz(&bytes) {
+                        Ok(_) => true,
+                        Err(e) => {
+                            logger(&format!("flirt: failed to parse .pat.gz {}: {}", name, e));
+                            false
+                        }
+                    },
+                    Err(e) => {
+                        logger(&format!("flirt: failed to read .pat.gz {}: {}", name, e));
+                        false
+                    }
+                }
             } else if lower.ends_with(".pat") {
                 match std::fs::read_to_string(p) {
                     Ok(text) => match builder.add_pat(&text) {
@@ -185,29 +193,18 @@ impl FlirtMatcher {
         let sig_count = set.len();
         if sig_count == 0 {
             return Err(Error::InvalidRuleFile(format!(
-                "flirt: no signatures loaded from {} ({} sources attempted, {} .pat.gz skipped)",
+                "flirt: no signatures loaded from {} ({} sources attempted)",
                 path.display(),
                 source_count,
-                gz_skipped
             )));
         }
 
-        if gz_skipped > 0 {
-            logger(&format!(
-                "flirt: loaded {} signatures from {} files in {} ({} .pat.gz skipped — gunzip to enable)",
-                sig_count,
-                source_count,
-                path.display(),
-                gz_skipped
-            ));
-        } else {
-            logger(&format!(
-                "flirt: loaded {} signatures from {} files in {}",
-                sig_count,
-                source_count,
-                path.display()
-            ));
-        }
+        logger(&format!(
+            "flirt: loaded {} signatures from {} files in {}",
+            sig_count,
+            source_count,
+            path.display()
+        ));
 
         Ok(Self {
             set,