Vector35 / binaryninja-api

Public API, examples, documentation and issues for Binary Ninja
https://binary.ninja/
MIT License
896 stars 200 forks source link

Incomplete switch case detection due to the presence of data variables in the middle of the jump table #4528

Open amyspark opened 1 year ago

amyspark commented 1 year ago

Version and Platform (required):

Bug Description:

FFmpeg uses inline assembly to create pure functions: https://github.com/FFmpeg/FFmpeg/blob/master/libavcodec/x86/mlpdsp_init.c#L149-L162

This syntax is understood by the MachO disassembler, but with COFF, the disassembler only understands the labels and not the underlying instructions.

Steps To Reproduce:

Compile FFmpeg with flto=thin, then open ffprobe.exe in Binary Ninja.

Expected Behavior:

Functions disassembled successfully.

Screenshots:

imagen imagen

Additional Information:

ffprobe.osx.zip

ffprobe.exe.zip

xusheng6 commented 1 year ago

The issue has nothing to do with pure functions. The code is just a regular switch case that we should be able to handle without issue. I suspect the issue is caused by the presence of the symbols -- the existing data vars might interfere with the jump table creation.

xusheng6 commented 1 year ago

This can be partially mitigated by setting UIDF on the index variables used by the switch case. Steps:

  1. Navigate to mlp_filter_channel_x86.lto_priv.0
  2. Switch to MLIL
  3. Select the r8 token on this line: 6 @ 14052ce2e int64_t r8 = sx.q(arg3)
  4. Right click, and click Set User Variable Value
  5. In the dialog that pops up, set the range to { 0 : 8 : 1 }. Then click Accept

Then do the same thing for the r9 variable, and set its range to {0: 5: 1}. This should fix the switch-case detection and get the relevant basic blocks properly disassembled, though the HLIL still looks a bit strange

Jakub259 commented 9 months ago

I've probably stumbled upon the same issue. 3.6.4658-dev switch: 3 6 4658-dev_switch 3.6.4658-dev jump table: 3 6 4658-dev_jump_table 3.4.4095-dev switch: 3 4 4095-dev_switch 3.4.4095-dev jump table: 3 4 4095-dev_jump_table

Archive with source code and the binary: files.zip

3.4.4095-dev decompiles correctly with no user interaction required.
3.6.4658-dev decompiles correctly after manually modifying the jump table type to adjust size.

xusheng6 commented 1 month ago

I had a look at this today and it is obvious that the presence of the data variables (probably from the symbol info) us causing issues. The jump table initially looks like this:

Screenshot 2024-07-30 at 11 10 30 AM

We can see the code is creating the jump table with only one element, probably because it sees there are other data variables behind it and it decides not to overwrite them. If I fix the jump table size manually it will work just fine:

Screenshot 2024-07-30 at 11 09 49 AM

xusheng6 commented 1 month ago

In the second case provided by Jakub259, the presence of data variable data_402018 prevents the analysis from figuring out the size of the jump table correctly:

Screenshot 2024-07-30 at 11 14 22 AM

If we set the jump table array to the correct size manually, or even simpler, just remove data_402018 and re-analyze the function, things will work

xusheng6 commented 1 month ago

I believe we should relax the restrictions on the jump table size detection and allow it to overwrite certain data variables