Vector35 / binaryninja-api

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

Breakpoints should have a setting which allows them to halt analysis or not #6123

Open SecondNewtonLaw opened 1 week ago

SecondNewtonLaw commented 1 week ago

Version and Platform (required):

Bug Description: Breakpoints stop function analysis; this is especially harmful when reverse engineering dumps of some programs, as sometimes authors will purposefully place breakpoints and break analysis throughout, and while I solved this manually on my PE dumper by byte patching the breakpoints, it is a pressing issue for any other reverse engineer that maybe does not want to write their own dumper, yet they're still faced with breakpoints throughout. This is worse considering __noreturn propagates to callers and int3 makes a function be __noreturn, so if I make a function simply have a breakpoint, I can break analysis in the binary in a substantial manner.

Steps To Reproduce:

Expected Behavior: The function should be analyzed completely, or if so, try to continue analysing after a breakpoint if some conditions are met; this could be done in a separate stage or after the initial analysis of the function is completed if it proves to be expensive enough.

Screenshots/Video Recording: N/A

Binary: N/A

Additional Information: IDA Pro handles breakpoints my simply placing __breakpoint() in its stead, which could be a manageable solution, as it also continues disassembling the function until it cannot continue. for whichever reason.

xusheng6 commented 3 days ago

I have seen both cases where we should and should not stop analysis at int3. Maybe we should at a configurable option for it

plafosse commented 3 days ago

I believe the behavior that we have currently is the best option, however we should allow a setting which can override this default behavior. This is unfortunately not an easy fix as its reliant on #551

Pixeluted commented 2 days ago

I believe the behavior that we have currently is the best option, however we should allow a setting which can override this default behavior. This is unfortunately not an easy fix as its reliant on #551

For some scenarios it might be the best, however for malware analysis it is making the reverse engineering much more harder as malware developers can simply put __debugbreak() and break Binary Ninja's analysis.

psifertex commented 2 days ago

They'll also break the application unless it's being debugged too! So if they're going to register an exception handler to handle it they might as well just put junk bytes, or ud2 or any number of invalid instructions as those will also stop analysis. It's basically impossible to generically solve this problem and so just making sure we give users the ability to modify analysis (which we already do) is the most important thing.