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

"Call graph nodes not sorted" #84

Open
dinedag opened this issue Nov 23, 2021 · 4 comments
Open

"Call graph nodes not sorted" #84

dinedag opened this issue Nov 23, 2021 · 4 comments

Comments

@dinedag
Copy link

dinedag commented Nov 23, 2021

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!

@cblichmann
Copy link
Member

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).

@dinedag
Copy link
Author

dinedag commented Nov 24, 2021

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
Copy link
Author

dinedag commented Nov 24, 2021

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.

@cblichmann
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants