Vector35 / binaryninja-api

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

call + pop pattern not handled by default #4752

Open xusheng6 opened 9 months ago

xusheng6 commented 9 months ago

In shellcode, this pattern is used quite often. It uses a combination of call + pop to load the address of a string into a register.

image

Note, the default behavior is even more problematic -- the disassembly would continue to disassemble the string "VirtualProtect" and produce garbage. To get what is shown in the image, I have already set the function to a no-return to stop the control flow.

The tricky part of this is there is no easy way to handle this. If we wish to patch the code, the space is not enough. We would have to create a new segment and put some code there, which is complex

xusheng6 commented 9 months ago

One person has used this example as support for having IDA-style define/undefine data/code, but I believe that is NOT the best way to handle this. That will break the continuity of control flow

Btw, a workflow should be able to handle this case

xusheng6 commented 9 months ago

Related to #4428

xusheng6 commented 9 months ago

We already handle the "call +5" case https://github.com/Vector35/arch-x86/blob/ce54c270537a36255608cd1f67df64f971558d0e/il.cpp#L1237-L1242. But this cannot be easily changed to handle "call + pop", since the lifter only has the info of the current instruction, it cannot know the target of the call is an pop instruction.

xusheng6 commented 2 months ago

While I was looking at https://github.com/Vector35/binaryninja-api/issues/5583, an idea suddenly strikes me, that setting the called function as no-return and then get it inlined might help with the situation. I created a monimal PoC and it seems to be working! As verified by the API, our analysis actually knows rdx is 0x5 after the pop

Screenshot 2024-06-17 at 1 35 03 PM

xusheng6 commented 2 months ago

oxfoo1m3.zip

A relevant binary

xusheng6 commented 3 weeks ago

Here is the database containing the code shown in the 1st screenshot in this issue:

ffmodule.exe.bndb.zip

The code originally looks like this:

Screenshot 2024-07-25 at 1 41 19 PM

Setting the function sub_1400173d4 to be inlined does seem to fix the code at the IL level. However, the disassembly still goes on wildly regardless:

Screenshot 2024-07-25 at 1 43 19 PM

Setting the function sub_1400173d4 to be inlined and no-return does fix the code at the IL level, but the disassembly is not continuing, which is not ideal:

Screenshot 2024-07-25 at 1 49 25 PM