Binary Ninja support draft#112
Draft
0cyn wants to merge 30 commits into
Draft
Conversation
Collaborator
|
Thank you a lot for taking the time to implement a full binary ninja plugin. We had it in the todo list for quite a while but never had the time to really put in the effort. As you noticed, the SELF_CONTAINED mode (or FULL mode) is a new concept for the v1.x release and it's still not implemented for any backend, so it's not a big deal to hold on to it for the time being. Just as a notice, we will need a few days to review the PR as it is covering substantial work |
Collaborator
|
😍 Thanks a lot for your contribution here, we have been willing to get a working implementation of Quokka for BN for some times. We will try to do our best to get this merged, but as the PR is quite large, it may take some times. Apologies in advance. |
Collaborator
|
Note for ourselves: BN gives free CI license for open source plugin if we want to integrate it in the CI ( source) |
Author
|
Moved this to a draft pending some cleanup as I'm very displeased w/ the state of the code currently and would like to spend some time making this quick sketch more maintainable 😄 . Glad to hear there is interest in adding it ❤️ |
Collaborator
|
I've taken the liberty of adapting a bit the plugin, aligning the coding style, fixing some bugs, adding tests, ... |
Binary Ninja loads folder plugins as regular Python packages, so the entry point can reach its implementation with a relative import instead of inserting the plugin directory into sys.path. The previous approach mutated the interpreter-wide sys.path shared by every loaded plugin and registered bn_quokka (and, through its import fallbacks, bare util and quokka_pb2 names) as top-level modules, risking collisions with other plugins or packages.
The headless CLI previously inserted the plugin directory itself into sys.path, which registered bn_quokka (and, through its import fallbacks, bare util and quokka_pb2 names) as importable top-level modules. Resolve the exporter through _load_export_file() instead: when the module is imported as part of the plugin package, the package-relative import is used directly; when executed as a standalone script, the package's parent directory is put on sys.path and the exporter is imported through the package's on-disk name, so its internals stay namespaced under the package.
The optional-import shims aliased BinaryView and Type to typing.Any when the BinaryNinja API is unavailable, which silently turned every annotation using them into Any for type checkers. Since all modules use deferred annotation evaluation (PEP 563), names that only appear in annotations are never looked up at runtime: import them under a TYPE_CHECKING guard instead so checkers always see the real types. The runtime fallback now covers only the names actually evaluated at runtime (enum members and isinstance checks), and those fall back to None uniformly; the code paths using them are unreachable without BinaryNinja, guarded by _require_binaryninja() or the binaryninja-is-None check in export_file().
The try/except ImportError blocks in bn_quokka existed only so the modules could be imported in environments without the BinaryNinja API (tests, CI). That is a test concern, so move it into the tests: a new tests/conftest.py installs a strict stub of the binaryninja modules in sys.modules before collection when the real API is absent. Only the names imported at module level are stubbed, so unexpected API use fails loudly instead of being absorbed by a permissive mock. The production modules now import binaryninja unconditionally, which removes the dead code the fallbacks dragged along: the None aliases, util.BN_AVAILABLE, _require_binaryninja(), and the binaryninja-is-None guard in export_file(). Without the API, importing bn_quokka now fails at the import statement with the actual ModuleNotFoundError; the headless CLI already defers that import and reports it cleanly. Tests needing the real API are marked requires_binaryninja and skipped when only the stub is present, replacing pytest.importorskip, which the stub would have satisfied incorrectly. The stub also fixes the pytest collection error for the package __init__.py, which imports binaryninja unconditionally.
Generate bn_quokka/quokka_pb2.py from the shared schema instead of committing it, following the Python bindings convention where setup.py generates bindings/python/quokka/quokka_pb2.py at build time and the file is git-ignored. The checked-in copy also hard-pinned the protobuf 6.31 runtime through its gencode version check while plugin.json only declares protobuf>=3.12.2; generating against the locally installed grpcio-tools keeps gencode and runtime in sync. - remove bn_quokka/quokka_pb2.py from the repository and git-ignore it next to the existing bindings entry - generate_proto.py reads proto/quokka.proto from the repository root directly (with a local proto/ fallback for standalone copies), replacing the proto symlink, which broke on Windows checkouts - install_dev.py generates the module before symlinking the plugin - the test conftest generates it on demand when grpcio-tools is available, and otherwise skips the extension tests with a warning, since even the package __init__ import chain needs the module - document the generation step in README.md and plugin.json
Pin grpcio-tools to 1.74.0 (ships protoc 31.1) so the generated quokka_pb2.py stays on the protobuf 31.x release line, the same line the Ghidra extension is constrained to by the protobuf-java bundled in the Ghidra release. Raise the runtime requirement in plugin.json to protobuf>=6.31,<7 accordingly: gencode produced by protoc 31.x requires a matching runtime, so the previous >=3.12.2 floor understated the real requirement. requirements.txt documents that it holds development/codegen dependencies (runtime ones live in plugin.json) and that the pin should be bumped together with the Ghidra submodule. Note: the IDA plugin (FetchContent v30.2) and the Python frontend (unpinned grpcio-tools, protobuf>=3.12.2 floor) are not yet on this line; aligning them is left for a separate change since it affects the release pipeline (frontend would need protobuf>=6.31 and Python>=3.9).
The BinaryNinja plugin manager installs a plain git tree (plugin.json at the root of the cloned repository) with no build or install hook, so unlike the Python bindings there is no artifact step where quokka_pb2.py could be generated outside the repository. Commit the generated module so end users only need the protobuf runtime declared in plugin.json. The problems that previously made the checked-in copy undesirable are both addressed: - drift: a new BinaryNinja Extension workflow regenerates the module with the pinned toolchain on every change to binaryninja_extension/ or proto/ and fails if the committed copy differs; it also runs the extension test suite - version mismatch: the module is generated by the grpcio-tools version pinned in requirements.txt (protoc 31.1), matching the protobuf>=6.31,<7 runtime requirement in plugin.json generate_proto.py, install_dev.py, and the test conftest keep working as regeneration entry points during development.
- rename the camelCase methods (resolve_segment_index, resolve_segment_offset, resolve_file_offset, is_address_initialized, resolve_type_index, export_type_to_type_refs) to snake_case per PEP 8 and the rest of the Python code in the repository - use PEP 604 unions (X | None, int | str) consistently instead of mixing Optional/Union with the | syntax already used in export_headless.py; all occurrences are annotations, which are never evaluated thanks to from __future__ import annotations, so the runtime Python floor is unchanged; drop the ModeInput alias and the now-redundant quoted annotations along the way - rename requirements.txt to requirements-dev.txt to make explicit that it holds codegen/test dependencies, while runtime dependencies live in plugin.json
It contained a single blank line. The repository root .gitignore already covers Python build artifacts, and the generated bn_quokka/quokka_pb2.py is deliberately committed, so there is nothing extension-specific left to ignore.
The try-relative/except-absolute import dance for util and quokka_pb2 only triggered when export.py or util.py were imported as top-level modules, with the bn_quokka directory itself on sys.path. Nothing does that anymore: the plugin entry point, the headless CLI, and the tests all import bn_quokka as a package, so plain relative imports suffice. The custom 'run generate_proto.py' ImportError is also obsolete: the generated module is committed and guarded by CI, so every checkout has it, and the stock ModuleNotFoundError names the missing module clearly in the remaining abnormal cases.
The plugin command previously ran the whole export synchronously in the command callback, freezing the BinaryNinja UI for the duration of update_analysis_and_wait() plus a full walk of the binary. Move the work into a BackgroundTaskThread, the BinaryNinja convention for long-running plugin work: - the task shows live progress text for each export phase and supports cancellation between phases; cancelling aborts before the output file is written - result dialogs are dispatched back to the UI thread via execute_on_main_thread - export_binary_view()/run_export_pipeline() gain an optional progress callback invoked before each phase; the callback may raise (the new ExportCancelled) to abort the export, keeping the pipeline itself free of any UI dependency - waiting for analysis is kept but now happens off the UI thread with explicit progress text, so invoking the command mid-analysis blocks nothing and still exports complete data The test stub provides a real BackgroundTaskThread class (the entry point subclasses it at import time, which a MagicMock cannot support), and new tests cover the task success, failure, and cancellation paths against the stub.
export.py had grown to ~1650 lines covering nine concerns. Split it along the semantic clusters of the schema while keeping bn_quokka.export as the stable import surface, so the plugin entry point, the headless CLI, tests, and any external script keep their imports unchanged (including the historical __all__): - context.py: ExportContext shared by all phases, ExportCancelled - exporters/binary.py: program image (Meta, Segment, Layout, Data) - exporters/types.py: type table, type xrefs, header collection - exporters/cfg.py: functions, basic-block splitting, edges - exporters/instructions.py: instruction/operand token machinery - exporters/references.py: cross-reference export and classification - export.py: pipeline orchestration and entry points only The import graph is a DAG (util <- context <- exporters <- export); helpers shared across phases became public in their owning module (extract_mnemonic, token_value, ... in instructions; map_calling_convention moved to util since both meta and cfg need it). All code moved verbatim apart from those visibility changes. README documents the new layout.
bn_quokka deliberately logs through Python's logging module so that headless runs can configure it normally (export_headless.py uses basicConfig), but inside the UI those records never reached the BinaryNinja log pane, making diagnostics such as unresolved types, skipped types, and hash failures invisible to users. Attach a logging.Handler in the plugin entry point that forwards records from this package's logger hierarchy to the BinaryNinja log (DEBUG->log_debug, INFO->log_info, WARNING->log_warn, ERROR/CRITICAL->log_error). The handler is installed only when the UI is available (core_ui_enabled), is idempotent across plugin reloads, and sets the package logger to INFO so the skipped-type diagnostics surface in the pane, which has its own per-level filtering. The library code keeps using stdlib logging and stays free of any UI dependency.
External and imported symbols get synthetic addresses from BinaryNinja that may lie outside every mapped segment. _set_address_fields() encoded such addresses as segment_index=0, segment_offset=0, and since the Python bindings reconstruct addresses as virtual_address(segment_index, segment_offset), every unresolved extern function collapsed onto segment 0's base address, colliding with each other and with whatever legitimately lives there. The IDA flow does not have this problem because IDA's loader materializes an extern segment. Mirror that: SegmentExporter now scans the external/imported symbol kinds and synthesizes SEGMENT_EXTERN pseudo-segments (permissions 0, no file backing) covering their unmapped addresses, via the new build_extern_segments() in util. Addresses are clustered per gap between real segments and each pseudo-segment is clamped to the next real segment, preserving the sorted non-overlapping invariant that find_segment_index's binary search relies on. External symbols that still resolve to no segment are skipped with a warning instead of being emitted at a colliding address, and the remaining segment-0 fallback in _set_address_fields logs the loss instead of staying silent. build_extern_segments is pure and covered by unit tests (clustering, clamping, resolution after merge).
Function edges were labelled purely by the block's out-degree (1 -> EDGE_JUMP_UNCOND, 2 -> EDGE_JUMP_COND, >2 -> EDGE_JUMP_INDIR), discarding the per-edge BranchType that BinaryNinja provides. A two-way jump table came out as conditional, and a conditional jump whose other target fell outside the function degraded to unconditional - data that diffing tools built on quokka rely on. _ExportBlock now carries (target address, BranchType) pairs end to end through the call-site block splitting; outgoing_targets and outgoing_edge_types became derived properties so the block-typing logic is unchanged. Edge emission maps each edge individually via _map_edge_type (True/False -> COND, Unconditional -> UNCOND, Indirect/Unresolved -> INDIR, Call/Syscall -> CALL, otherwise UNKNOWN), and the loop is extracted into _export_edges so it can be tested against a real protobuf message. Synthetic fall-through edges introduced by the call-site splits keep their explicit UnconditionalBranch type, identical to before. Tests cover the type mapping, the derived block views, a multi-way indirect block, and the previously-degrading single-surviving-conditional case.
TypeExporter promotes primitive types carrying a registered_name (e.g. typedef uint32_t DWORD) to TYPEDEF entries, but resolve_type_index checked map_primitive_type first, so every use of the alias (struct members, data variables, pointer/array element types) resolved to the raw primitive index, leaving the exported typedef entry orphaned with no inbound references. The promotion rule now lives once in util.is_named_primitive_alias and both sides apply it: registration as before, and resolve_type_index prefers the registered TYPEDEF entry for alias uses, falling back to the primitive index when the alias was never registered (no new TYPE_UNK regressions). The one place that must NOT follow the alias is the promoted typedef's own element type: resolving it through the alias would make the entry point at itself. resolve_type_index grows an unaliased keyword for exactly that call site in _build_reference_composite. The test stub now provides Type/Segment/Section as real classes so isinstance-based validation works, enabling unit tests over synthetic Type objects: alias detection, alias uses resolving to the typedef entry, the element type resolving through to the primitive, and the unregistered-alias fallback.
collect_headers() sorted declarations alphabetically, so a struct used by another could be declared after its user and the resulting header could not be compiled or parsed as a unit. Use TypePrinter.default.print_all_types - the machinery behind the UI's Export Header feature - which emits forward declarations and orders definitions by dependency. The previous implementation remains as a documented fallback (alphabetical, not guaranteed to compile) for environments where TypePrinter is unavailable or fails; it is reached via getattr so older BinaryNinja versions keep working without it. Tests cover the printer path (ordering preserved verbatim, sorted named-types input, view passthrough), the fallback without TypePrinter, the fallback on printer failure, and the empty view.
Four heuristics flagged in review, addressed together because they
share infrastructure:
- ExportContext.instruction_branches() caches one normalized decode
(length, branch list) per address. _is_call_site previously re-read
and re-decoded every instruction during function export and
_branch_targets did it all again during reference export; both now
share one decode per instruction.
- _block_type() inferred conditional returns from substring matches on
rendered instruction text ('ret' in ..., ',pc' in ...), which any
symbol containing 'ret' could satisfy. It now checks the last
instruction's branch info for FunctionReturn combined with the
block's conditional flow - structural and architecture-independent.
- _is_call_site() falls back to the lifted IL (LLIL_CALL/SYSCALL/
TAILCALL) for indirect calls instead of an x86/ARM/MIPS mnemonic
whitelist.
- Operand access is exported as ACCESS_UNKNOWN (named constants
replace the magic 1/2/3): BinaryNinja tokens carry no access
information and the previous x86-only mnemonic table produced wrong
metadata for every other architecture. The token-based reference
classifier loses its dependence on that table and conservatively
reports READ for memory operands; writes are caught by the IL pass.
- _classify_nonbranch_reference() walks the LLIL expression tree
structurally (constant in a store destination -> WRITE, in a load
source -> READ, feeding a call -> READ) instead of substring-matching
the IL's string rendering, and runs before the token fallback since
it is the most precise signal. The stringified-LLIL parsing is gone.
Tests cover the decode cache, structural CNDRET (including the
ret-substring false positive), direct/indirect/negative call sites,
ACCESS_UNKNOWN operands, the LLIL walk (store dest, load src, stored
value, call target, unrelated constant), and the token classifier.
_populate_data_xrefs() scanned every reference for every data record, making data export O(data x references) - intractable on large binaries (1e5 data items x 1e6 references). Build sorted (address, reference index) lists for source and destination endpoints once per export and find each record's cross-references with a bisect range lookup, bringing the phase to O(R log R + D(log R + k)). Address-bearing endpoints only; type references (data_type_identifier oneof) are excluded exactly as before, and the matched indices are emitted in ascending reference order so the output is byte-identical to the previous implementation. Tests cover index construction, oneof exclusion, half-open range lookups, record matching at the boundaries, and output ordering. This closes the performance pair from review: the other half (full binary re-decoding repeated across FunctionExporter and ReferenceExporter) was addressed by ExportContext.instruction_branches in the heuristics commit.
The smoke tests parsed the raw protobuf but never loaded it through
the Python bindings, which are the real compatibility contract - a
quokka.Program() load would have caught the extern segment-0 collision
directly.
Two layers, following the existing IDA/Ghidra structure:
- tests/python/tests/offline/test_binja_export.py mirrors the offline
Ghidra fixture tests: it loads tests/dataset/qb-crackme_binja.quokka
through quokka.Program and checks structural properties (backend,
functions, blocks, primitive type table, segment ordering) plus a
regression test asserting all reconstructed function addresses are
unique (the extern collapse failure mode). It skips until the
fixture is generated (BinaryNinja required) and then runs in the
existing python.yml job; the regeneration command is documented in
the module docstring:
python binaryninja_extension/export_headless.py docs/samples/qb-crackme \
-o tests/dataset/qb-crackme_binja.quokka
- the extension smoke tests gain a live round-trip on machines with
BinaryNinja: export qb-crackme, load it with quokka.Program, check
fun_names and unique function addresses.
The exporter version was hardcoded in MetaExporter and again in plugin.json, adding two locations to the release checklist. It now lives once in bn_quokka/version.py (mirroring bindings/python/quokka/version.py): MetaExporter stamps it into exporter_meta.version, the package re-exports it, and a new test asserts plugin.json declares the same version, so the JSON cannot drift - it fails CI instead. ExportContext also took a file argument that nothing ever used (export_binary_view passed a throwaway io.BytesIO()); the parameter and the dead output_file/file attributes are gone, along with the now-unneeded io imports.
The installation and usage pages only covered the Python bindings and the IDA plugin, and the README listed IDA Pro and Ghidra as the only backends. - README: Binary Ninja added to the backend list and architecture diagram, an installation section pointing at the extension README, and a headless export example with the commercial-license note - docs/installation.md: BinaryNinja Extension section with requirements (protobuf runtime in Binary Ninja's interpreter), install_dev.py and manual install paths, and a note that the committed quokka_pb2.py removes any generation step for users - docs/usage.md: Binary Ninja export section covering the UI command (background task with progress/cancel) and the headless CLI with its options
- realign the architecture diagram under the widened three-backend top - simplify the protobuf requirement wording in the installation page; plugin.json remains the authoritative version constraint - keep the Ghidra README pointer with the Ghidra headless section (the Binary Ninja insertion had landed between the two) and close the Binary Ninja section with a pointer to its own README
- type: the extension is an export utility, not a core-analysis plugin; categorize it as helper - minimumbinaryninjaversion: 3000 predates even Binary Ninja 3.0 stable (build 3233) and was never tested. Declare 4911 (Binary Ninja 4.0): every API used (BackgroundTaskThread progress/cancellation, execute_on_main_thread, core_ui_enabled, Type.registered_name, NamedTypeReference.target, guarded TypePrinter) is available there, and the protobuf>=6.31 runtime requires the Python >= 3.9 bundled with modern builds - README: document that the plugin installs manually and cannot be listed in the official plugin manager from this repository, since the plugin manager requires plugin.json at the repository root; a dedicated distribution repository would lift that limitation
Running the export pipeline against a real Binary Ninja (5.4) exposed a load failure the stubbed tests could not catch: quokka.Program() raised KeyError: 0 because CallingConvention.from_proto in the Python bindings has no entry for CC_UNK, even though the enum defines UNKNOWN, and program.py reads meta.calling_convention unconditionally. The IDA exporter never emits CC_UNK, so this was latent until now. Two-sided fix: - bindings: map CC_UNK to CallingConvention.UNKNOWN so any proto-legal value loads - exporter: map Binary Ninja's 'sysv' calling convention (the System V AMD64 ABI, the default on linux-x86_64 platforms) to CC_CDECL, which is what the IDA exporter emits for the same binaries, and 'win64' (Microsoft x64) to CC_FASTCALL, so common platforms no longer fall back to CC_UNK at all Verified against Binary Ninja 5.4.9744-dev: the full extension suite passes (20 passed, stub-only tests skipped), and the whole offline bindings suite (124 tests) still passes.
Generated with the headless export script against Binary Ninja
5.4.9744-dev:
python binaryninja_extension/export_headless.py docs/samples/qb-crackme \
-o tests/dataset/qb-crackme_binja.quokka
This activates the 13 offline bindings tests in
tests/python/tests/offline/test_binja_export.py (all passing),
including the regression test asserting extern/imported functions
resolve to unique addresses.
Merge the sysv branch into cdecl and win64 into fastcall - same mapping, fewer branches and shorter comments.
patacca
force-pushed
the
binaryninja_support
branch
from
ee39a33 to
b14ad2d
Compare
The extension's test directory mixed two kinds of tests: BN-free unit tests (run in CI against the conftest stub) and integration tests requiring a real Binary Ninja, which were marker-skipped in CI. The repository already has a convention for the latter: per-backend directories under tests/python/tests/ (ida/, ghidra/) that skip automatically when the tool is absent. Follow it: the three export integration tests (LIGHT, SELF_CONTAINED, and the quokka.Program round-trip with the extern address-uniqueness regression) move to tests/python/tests/binja/, gated by a module-level importorskip on binaryninja. The extension suite is now fully BinaryNinja-free - 46 passed, zero skips in CI - and the now-unused requires_binaryninja marker machinery is removed from its conftest. Verified in all four modes: extension suite 46 passed (stub) / 17 passed + 29 stub-only skips (real BN); binja integration dir 1 module skip without BN / 3 passed against Binary Ninja 5.4.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hi! I wanted to look at adding Binary Ninja support for this tool, and gave it a basic shot in this PR.
I wanted to offer to submit this upstream so this program could have first class support for it if desired.
This doesn't implement SELF_CONTAINED currently as it doesn't seem to be implemented for the other plugins, but that can totally be done.
If you have any opinions/questions on the code or structure please let me know, I'd be glad to do whatever is wanted to get this merged :)