Closed Bug 1682304 Opened 3 years ago Closed 3 years ago

REDIRECT_TO_NOOP_ENTRYPOINT of a dll injected via import address table may result in firefox crash if it's also injected with SetWIndowsHookEx

Categories

(External Software Affecting Firefox :: Other, defect, P3)

Tracking

(Not tracked)

RESOLVED WORKSFORME

People

(Reporter: arthur.vaiselbuh, Unassigned)

References

Details

Crash Data

Attachments

(1 file)

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/87.0.4280.88 Safari/537.36

Steps to reproduce:

Inject a dll both through import address table and using SetWindowsHookEx(for example with WH_CALLWNDPROC).
Launch firefox.
Since firefox version 84 IAT injection the dll entry point is redirected to no-op entry point, preventing initialization. - see bug 1659438
Windows recognizes the dll has been loaded, and calls the hooked function. If the hooked function depends on initialization - for example refers to a global pointer that is initialized in dllmain - the process will crash.

Actual results:

Firefox fails to open entirely - process is started and immediately crashes calling winfault. Attached the call stack of the crash(last call is to an uninitialized function pointer)

Expected results:

I dont think it's necessarily a firefox bug, but I'm pretty sure it's unintended behavior since preventing dll loading is supposed to help with stability, and not crash the process ( for example, if RedirectToNoOpEntryPoint fails the dll is loaded anyway to prevent crashing the process).

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Component: Untriaged → Crash Reporting
Product: Firefox → Toolkit

Which application causes this? I understand RedirectToNoOpEntryPoint relies on their implementation, which is not a good design actually.

Flags: needinfo?(arthur.vaiselbuh)
Severity: -- → S3
Component: Crash Reporting → Other
Priority: -- → P3
Product: Toolkit → External Software Affecting Firefox
Version: Firefox 84 → unspecified

The application is "IBM security Trusteer Rapport", though I'm sure it's not the only program and scenario that RedirectToNoOpEntryPoint will create issues with.

Flags: needinfo?(arthur.vaiselbuh)
See Also: → 1678406
See Also: → 1659438

As commented in bug 1659438, we have prevented dll injection va Import Table though our protection did not prevent all attacks. One of the reason why IAT tampering is considered differently is if a process fails to load a dependent module for whatever reason, the process will never launch, and we cannot block it. For the same reason, bug 1659438 was required to enable CIG. Please understand we need to have some control on injected modules to keep our process safe.

It seems you're working for IBM. Do you have any contact to IBM Security Trusteer Rapport team? I'm wondering they can achieve their functionality some other way, or at least avoid this kind of crash.

(In reply to Arthur from comment #0)

Windows recognizes the dll has been loaded, and calls the hooked function. If the hooked function depends on initialization - for example refers to a global pointer that is initialized in dllmain - the process will crash.

If the hooked function depends on initialization in DllMain, can you make the hooked function do nothing when no initialization happened, for instance?

Flags: needinfo?(arthur.vaiselbuh)

Sorry for the delayed response.

I am from the Rapport team, and we've taken measures to prevent the crash from our end on the day version 84 was publicly released (it might still crash for users until they receive an update from our servers).

I just want to emphasize that REDIRECT_TO_NOOP_ENTRYPOINT prevents not only the call to dllmain but also the call to the CRT entry point - so initialization for static variables everywhere will not be called, and the CRT initialization function will not be called as well, which might cause unexpected bugs in any program which attempts this type of injection.

This bug report does not aim to fix a specific interaction problem with Trusteer Rapport, it was to bring to your attention the possible pitfalls of REDIRECT_TO_NOOP_ENTRYPOINT, possibly for other programs as well. If the conflict is with rapport only, or if your decision is to keep using this redirect then we will work around it on our end.

Flags: needinfo?(arthur.vaiselbuh)

(In reply to Arthur from comment #5)

Sorry for the delayed response.

I am from the Rapport team, and we've taken measures to prevent the crash from our end on the day version 84 was publicly released (it might still crash for users until they receive an update from our servers).

I just want to emphasize that REDIRECT_TO_NOOP_ENTRYPOINT prevents not only the call to dllmain but also the call to the CRT entry point - so initialization for static variables everywhere will not be called, and the CRT initialization function will not be called as well, which might cause unexpected bugs in any program which attempts this type of injection.

This bug report does not aim to fix a specific interaction problem with Trusteer Rapport, it was to bring to your attention the possible pitfalls of REDIRECT_TO_NOOP_ENTRYPOINT, possibly for other programs as well. If the conflict is with rapport only, or if your decision is to keep using this redirect then we will work around it on our end.

Thank you for sharing your opinion. Fortunately we don't receive any compat issues other than Rapport so far. I really understand your concern, but our message here is Import Table tampering should not be used because we cannot simply block it from being loaded. We might implement another way to stop this type of injection in the long term, but for this case we'd like you to consider to implement a workaround on Rapport side.

Please understand our goal is not to shut out everyone. We appreciate if a third-party application provides additional values to users, but it's also the fact that we have had a lot of compat issues caused by DLL injection and we cannot leave it. I hope we can find a way to cooperate with third-party vendors.

Crash Signature: [@ @0x0 | fnHkINLPCWPSTRUCTW]

Closing because no crashes reported for 12 weeks.

Status: UNCONFIRMED → RESOLVED
Closed: 3 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: