Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Binary Ninja: diverging instructions not being treated as such #100

Closed
comex opened this issue Oct 16, 2022 · 1 comment
Closed

Binary Ninja: diverging instructions not being treated as such #100

comex opened this issue Oct 16, 2022 · 1 comment
Labels
bug Something isn't working

Comments

@comex
Copy link

comex commented Oct 16, 2022

Instructions such as ret are being treated as if they flow into the next instruction.

Instruction::Instruction automatically sets FLAG_FLOW if next_instruction is valid. In the IDA exporter, next_instruction is chosen by looking for xrefs from the current instruction, so diverging instructions would have next_instruction == 0, which is invalid, causing FLAG_FLOW to not be set. But in the Binary Ninja exporter, we just have:

  // TODO(cblichmann): Is this always the case in Binja?
  const Address next_instruction = address + instruction.length;

So it always gets set.

The code does already check for regular control flow later on, so it might be easiest to just reset FLAG_FLOW at that point:

--- a/binaryninja/main_plugin.cc
+++ b/binaryninja/main_plugin.cc
@@ -245,7 +245,9 @@ void AnalyzeFlow(
   if (has_flow) {
     // Regular code flow
     entry_point_manager->Add(instruction->GetNextInstruction(),
                              EntryPoint::Source::CODE_FLOW);
+  } else {
+    instruction->SetFlag(FLAG_FLOW, false);
   }
 
   const std::vector<BinaryNinja::ReferenceSource> xrefs =
@cblichmann cblichmann added the bug Something isn't working label Oct 17, 2022
copybara-service bot pushed a commit that referenced this issue Oct 20, 2022
… functions

This addresses #99 (`TYPE_SWITCH` xrefs are backwards) and #100 (diverging
instructions not being treated as such).

The plugin now goes to great lengths to figure out code references and distinguish them
from data references ([BinaryNinja API issue 3559](Vector35/binaryninja-api#3559)).

Note: I might refactor the plugin so that instead of decoding instructions one
by one, we rely on Binary Ninja's analysis and simply iterate over functions
their basic blocks.
PiperOrigin-RevId: 482437008
Change-Id: Ifb6f130dc35e1bb7df88db38ad3ea617d95b3aa8
@cblichmann
Copy link
Member

This should be fixed now. Thanks for the report.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants