fix: add return after zero-offset yield in extract_insn_offset_features

`extract_insn_offset_features` in the x86/x64 BinExport2 extractor handled
zero-offset patterns (e.g. `mov [reg], reg`) in a nested branch but was
missing a `return` after yielding `Offset(0)` and `OperandOffset(0)`.
Execution then fell through to the general `mask_immediate` path, which read
`immediate` from the last-matched expression node (a register, not an
integer). Since that field defaults to 0, the function emitted duplicate
`Offset(0)` and `OperandOffset(0)` features for every such instruction.

Fix: add `return` after the two yields in the zero-pattern branch.

Tests: add `FEATURE_COUNT_TESTS_BE2_INTEL` covering `MOV [EDI], CX` at
0x401125 in mimikatz, asserting each of `Offset(0)` and `OperandOffset(1,0)`
is emitted exactly once.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Willi Ballenthin
2026-04-22 18:28:16 +03:00
co-authored by Claude Sonnet 4.6
parent f8e0ea7144
commit 2d008fb53e
4 changed files with 29 additions and 0 deletions
+1
View File
@@ -11,6 +11,7 @@
-
### Bug Fixes
- fix: add return after zero-offset yield in extract_insn_offset_features so Offset(0) is not emitted twice @williballenthin
- fix: use f-string in binexport2 extractor so unexpected global feature value appears in ValueError message @williballenthin
- fix: correct scale/displacement expressions in get_operand_phrase_info 5-expression branch (used expression3 operator instead of expression4 value) @williballenthin
- fix: use HasField instead of truthiness in _index_vertex_edges so call-graph edges to/from vertex 0 are not silently dropped @williballenthin
@@ -136,6 +136,7 @@ def extract_insn_offset_features(
yield Offset(0), ih.address
yield OperandOffset(match.operand_index, 0), ih.address
return
value = mask_immediate(fhi.arch, match.expression.immediate)
if is_address_mapped(be2, value):
+16
View File
@@ -1544,6 +1544,22 @@ FEATURE_COUNT_TESTS_GHIDRA = [
("mimikatz", "function=0x401000", capa.features.basicblock.BasicBlock(), 3),
]
FEATURE_COUNT_TESTS_BE2_INTEL = [
# 0x401125: MOV [EDI], CX -- matches OFFSET_ZERO_PATTERNS, must yield Offset(0) exactly once
(
"mimikatz",
"function=0x40105d,bb=0x401125,insn=0x401125",
capa.features.insn.Offset(0),
1,
),
(
"mimikatz",
"function=0x40105d,bb=0x401125,insn=0x401125",
capa.features.insn.OperandOffset(1, 0),
1,
),
]
def do_test_feature_presence(get_extractor, sample, scope, feature, expected):
extractor = get_extractor(sample)
+11
View File
@@ -446,3 +446,14 @@ def test_binexport_feature_counts_ghidra(sample, scope, feature, expected):
sample = sample.parent / "binexport2" / (sample.name + ".ghidra.BinExport")
assert sample.exists()
fixtures.do_test_feature_count(fixtures.get_binexport_extractor, sample, scope, feature, expected)
@fixtures.parametrize(
"sample,scope,feature,expected",
fixtures.FEATURE_COUNT_TESTS_BE2_INTEL,
indirect=["sample", "scope"],
)
def test_binexport_feature_counts_intel(sample, scope, feature, expected):
sample = sample.parent / "binexport2" / (sample.name + ".ghidra.BinExport")
assert sample.exists()
fixtures.do_test_feature_count(fixtures.get_binexport_extractor, sample, scope, feature, expected)