fix: use instruction_indices in is_security_cookie for single-instruction basic blocks

`is_security_cookie` computes the last address in a terminal basic block by
iterating `bb.instruction_index` and indexing `ir.end_index - 1`. The BinExport2
protobuf spec omits `end_index` for single-element ranges, so protobuf returns 0
as the default. `0 - 1 = -1`, and -1 is not a key in `insn_address_by_index`,
raising `KeyError`.

Use `BinExport2Index.instruction_indices` to enumerate instruction indices, which
already handles single-instruction ranges via `HasField("end_index")`.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Willi Ballenthin
2026-04-22 18:43:05 +03:00
co-authored by Claude Sonnet 4.6
parent 6f96fc7d90
commit ec3aa4525d
3 changed files with 96 additions and 2 deletions
+1
View File
@@ -11,6 +11,7 @@
-
### Bug Fixes
- fix: use instruction_indices in is_security_cookie to handle single-instruction basic blocks where end_index is omitted, preventing KeyError on -1 @williballenthin
- fix: guard get_operand_expressions against empty expression tree so Ghidra-exported BinExport2 files with empty operands no longer raise IndexError @williballenthin
- 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
@@ -176,7 +176,7 @@ def is_security_cookie(
return True
# or insn falls at the end before return in a terminal basic block.
if basic_block_index not in (e.source_basic_block_index for e in flow_graph.edge):
last_addr: int = max(idx.insn_address_by_index[ir.end_index - 1] for ir in bb.instruction_index)
last_addr: int = max(idx.insn_address_by_index[i] for i in idx.instruction_indices(bb))
if instruction_address > last_addr - SECURITY_COOKIE_BYTES_DELTA:
return True
return False
+94 -1
View File
@@ -23,7 +23,15 @@ import fixtures
from google.protobuf.json_format import ParseDict
import capa.features.extractors.binexport2.helpers
from capa.features.extractors.binexport2 import BinExport2Index
from capa.features.extractors.binexport2 import (
AddressSpace,
MemoryRegion,
AnalysisContext,
BinExport2Index,
FunctionContext,
BasicBlockContext,
BinExport2Analysis,
)
from capa.features.extractors.binexport2.helpers import (
BinExport2InstructionPattern,
BinExport2InstructionPatternMatcher,
@@ -36,6 +44,7 @@ from capa.features.extractors.binexport2.helpers import (
)
from capa.features.extractors.binexport2.extractor import BinExport2FeatureExtractor
from capa.features.extractors.binexport2.binexport2_pb2 import BinExport2
from capa.features.extractors.binexport2.arch.intel.insn import is_security_cookie
from capa.features.extractors.binexport2.arch.arm.helpers import is_stack_register_expression
from capa.features.extractors.binexport2.arch.intel.helpers import get_operand_phrase_info
@@ -677,3 +686,87 @@ def test_get_operand_phrase_info_index_scale_displacement():
assert phrase.displacement.type == BinExport2.Expression.IMMEDIATE_INT
assert phrase.displacement.immediate == 4202519
assert phrase.base is None
def test_instruction_indices_single_insn_range():
be2_dict: dict[str, Any] = {
"instruction": [
{"address": 0x1000, "raw_bytes": b"\x90"},
],
"basic_block": [
{"instruction_index": [{"begin_index": 0}]},
],
}
be2 = ParseDict(be2_dict, BinExport2())
idx = BinExport2Index(be2)
bb = be2.basic_block[0]
assert not bb.instruction_index[0].HasField("end_index")
assert list(idx.instruction_indices(bb)) == [0]
def _make_security_cookie_ctx(xor_addr: int, bb_index: int, is_terminal: bool):
be2 = BinExport2()
section = be2.section.add()
section.address = 0x1000
section.size = 0x100
section.flag_x = True
expr_eax = be2.expression.add()
expr_eax.type = BinExport2.Expression.REGISTER
expr_eax.symbol = "eax"
expr_esp = be2.expression.add()
expr_esp.type = BinExport2.Expression.REGISTER
expr_esp.symbol = "esp"
op0 = be2.operand.add()
op0.expression_index.append(0)
op1 = be2.operand.add()
op1.expression_index.append(1)
mnem = be2.mnemonic.add()
mnem.name = "xor"
insn = be2.instruction.add()
insn.address = xor_addr
insn.raw_bytes = b"\x31\xc4"
insn.mnemonic_index = 0
insn.operand_index.extend([0, 1])
bb = be2.basic_block.add()
ir = bb.instruction_index.add()
ir.begin_index = 0
fg = be2.flow_graph.add()
fg.entry_basic_block_index = 0
fg.basic_block_index.append(0)
if not is_terminal:
edge = fg.edge.add()
edge.source_basic_block_index = 0
edge.target_basic_block_index = 0
idx = BinExport2Index(be2)
buf = b"\x00" * 0x200
analysis = BinExport2Analysis(be2, idx, buf)
address_space = AddressSpace(base_address=0x1000, memory_regions=(MemoryRegion(0, buf),))
ctx = AnalysisContext(
sample_bytes=buf,
be2=be2,
idx=idx,
analysis=analysis,
address_space=address_space,
)
fhi = FunctionContext(ctx=ctx, flow_graph_index=0, format=set(), os=set(), arch=set())
bbi = BasicBlockContext(basic_block_index=bb_index)
return fhi, bbi, be2.instruction[0]
def test_is_security_cookie_single_insn_terminal_bb():
xor_addr = 0x1000
fhi, bbi, insn = _make_security_cookie_ctx(xor_addr, bb_index=0, is_terminal=True)
result = is_security_cookie(fhi, bbi, xor_addr, insn)
assert isinstance(result, bool)