Vector35 / binaryninja-api

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

False positives in Imported Function Recognizers #4469

Closed alexrp closed 1 year ago

alexrp commented 1 year ago

Version and Platform (required):

Bug Description: Some functions are being marked as import thunks despite clearly doing more than that.

No amount of messing around with symbols and data vars in the API seems to make BN stop marking them as such.

Steps To Reproduce:

  1. Open this BNDB.
  2. Navigate to these functions: Sleep, InitializeSListHead, RaiseException, SetUnhandledExceptionFilter

Expected Behavior: These 4 functions should not be marked as import thunks.

Screenshots: image image image image

plafosse commented 1 year ago

This I assume is one of our function recognizer being overly broad.

plafosse commented 1 year ago

Thanks for the report this should be fixed in 3.5.4391

alexrp commented 1 year ago

@plafosse maybe I'm missing something, but this doesn't appear to be fixed on my end. I updated to 3.5.4392-dev Personal (f9a3e2ad) and navigated to the misidentified InitializeSListHead. I first undefined the function and then the kernel32:InitializeSListHead variable. Then I recreated the function, and it's once again identified as InitializeSListHead and marked as a library function. The same happens for SetUnhandledExceptionFilter and Sleep. It's a bit harder to tell with RaiseException because undefining it causes BN to consider it as an extension of FOutputDeviceWindowsError::method_001 (0x7ff69a9399d0).

plafosse commented 1 year ago

For me it looks like this:

image

Could you try re-opening the original binary?

plafosse commented 1 year ago

Also for SetUnhandledExceptionFilter all the calls to it seem to be un-named

image

Same with InitializeSListHead

image

I really believe I fixed the underlying issue here as I had to update our unit tests to remove one of the false positives that was showing up.

alexrp commented 1 year ago

Could you try re-opening the original binary?

Hmm, it does seem to work as it should when creating a new BNDB.

Is there any way to fix it for my existing BNDB? At this time, Import From BNDB doesn't carry over enough of my analysis progress for it to be a realistic option.

alexrp commented 1 year ago

Is there any way to fix it for my existing BNDB?

After messing around with the API a bit, I managed to get rid of the stale ImportedFunctionSymbols for good:

image

plafosse commented 1 year ago

ok great!