binexport icon indicating copy to clipboard operation
binexport copied to clipboard

"Call graph nodes not sorted"

Open dinedag opened this issue 4 years ago • 4 comments

Hey,

I am using BinExport with Ghidra 10.0.4 and am having an issue with getting the export files to work with BinDiff due to the following error:

bindiff --primary patch.BinExport --secondary vuln.BinExport
BinDiff 7 (@377901646, Jun  7 2021), (c)2004-2011 zynamics GmbH, (c)2011-2021 Google LLC.
Error: Call graph nodes not sorted: 45600998 >= 47509DF8

I note your commit on May 14th but there still seems to be an issue with the behaviour here. For reference, I've also replicated the same issue with previous versions of Ghidra.

Any idea what the issue might be? I do indeed have functions renamed in the way your commit mentions. But BinExport doesn't seem to be skipping them as I thought it now should.

Thanks!

dinedag avatar Nov 23 '21 11:11 dinedag

Hey there,

The actual check happens in BinExport2Builder.java:279. The TL;DR is that BinExport should skip every "non-loaded" function.

Are you able to share a file/project where this happens? To get you unstuck, you could hack the binexport2dump tool to filter out addresses and save it under a new name (or do this in the Java extension itself).

cblichmann avatar Nov 23 '21 12:11 cblichmann

Hey,

Seems like the issue I am having is then is that the functions are loaded and thus are still being included, which is kind of annoying... I managed to get over this issue by adding another extremely hacky check based on the function names content for a specific string.

      if (func.getName().contains("")) {
    	  continue;
      }

Unfortunately, it brings me straight into another error.

DetachFlowGraph: coudn't find call graph node for flow graph 00100420
Error: AttachFlowGraph: couldn't find call graph node for flow graph 00100420

Correct me if I am wrong but this error is saying, "I have a flow graph for this address! But there is no associated call graph" is that correct? If that is the case, I would expect I can just implement another hacky check based on the address when looping the functions in buildFlowGraphs and buildCallGraphs?

I'll see if I can share the binaries. Thanks for your prompt response!

dinedag avatar Nov 24 '21 09:11 dinedag

I've done some more digging on this and it looks like the issue is being caused by a jumptable. Opening the binary in Ghidra and jumping to that address reveals the following.

        LAB_00100420+1                                  XREF[1,1]:   4034d538(*), 4034df70(*)  
        LAB_00100420
        00100420 05 ed 05 0a     vstr.32    s0,[r5,#-0x14]
        00100424 06 28           cmp        r0,#0x6
        00100426 06 47           bx         r0

void UndefinedFunction_00100420(undefined4 param_1,code *UNRECOVERED_JUMPTABLE)

{
  int unaff_r5;
  
  *(undefined4 *)(&DAT_ffffffec + unaff_r5) = param_1;
                    /* WARNING: Could not recover jumptable at 0x00100426. Too many branches */
                    /* WARNING: Treating indirect jump as call */
  (*UNRECOVERED_JUMPTABLE)();
  return;
}

My thought is that when BinExport iterates over all the instructions for building blocks, it is picking up these instructions and then building a block for them but isn't able to build an associated flow graph and call graph?

What is weird however is the address in question does not show up in the function buildFlowGraph, in order to confirm this I added in a section of logging code and then grepped the output for the given address, so I don't quite understand the error BinDiff is providing. I also added a similar log statement for each instruction in buildInstructions

Address checkAddr = func.getEntryPoint();
String check = String.valueOf(checkAddr);
Msg.info(this, String.format("[Flow Graph] %s built!", check)); 

String check = String.valueOf(address);
Msg.info(this, String.format("[Instriction] %s built!", check));
➜  .ghidra_10.0.4_PUBLIC cat application.log* | grep "Flow Graph 100420"    
➜  .ghidra_10.0.4_PUBLIC 

➜  .ghidra_10.0.4_PUBLIC cat application.log* | grep "00100420"
2021-11-24 16:17:08 INFO  (BinExport2Builder) [Instriction] 1200100420 built!  
2021-11-24 16:08:01 INFO  (BinExport2Builder) [Instriction] 1200100420 built! 

Appreciate it is hard to help without the binaries but these are private so I'm trying to avoid getting to the point of needing to share them. Any guidance you can provide here would be much appreciated.

dinedag avatar Nov 24 '21 16:11 dinedag

I think the culprit lies with

/* WARNING: Treating indirect jump as call */

and Ghidra creating functions. You could just undefine the code in the function and see if that helps with the export.

Ultimately, BinExport should detect this issue and skip all of these new functions. But I have actually no idea how to get to this info from Ghidra's API.

cblichmann avatar Nov 25 '21 12:11 cblichmann