1587539 - Launcher Process is disabled when IObit Malware Fighter 5 is installed

Status: RESOLVED FIXED

(Firefox :: Launcher Process, defect, P1)

Description

[Toshihito Kikuchi [:toshi]]

Finally I found out a real scenario how the failure of GetIATThunksForModule("ntdll.dll"), one of the top launcher failures, happens.

I got a stable repro when:

• IObit Malware Fighter (= IMF) 5 is installed
• [Surfing Protection & Ads Removal] under [Browser Protect] is turned on

Here is the detail:

When IMF's driver IMFDownProtect.sys is loaded, it registers a load-image callback routine via PsSetLoadImageNotifyRoutine.

0: kd> knL
 # Child-SP          RetAddr           Call Site
00 ffff9380`99e148c8 fffff803`111e11b3 nt!PsSetLoadImageNotifyRoutine
01 ffff9380`99e148d0 fffff803`0af0eec6 IMFDownProtect+0x11b3
02 ffff9380`99e14950 fffff803`0af0e8fe nt!IopLoadDriver+0x4c2
03 ffff9380`99e14b30 fffff803`0a8bd645 nt!IopLoadUnloadDriver+0x4e
04 ffff9380`99e14b70 fffff803`0a92a715 nt!ExpWorkerThread+0x105
05 ffff9380`99e14c10 fffff803`0a9c886a nt!PspSystemThreadStartup+0x55
06 ffff9380`99e14c60 00000000`00000000 nt!KiStartSystemThread+0x2a
0: kd> u @rcx l2
IMFDownProtect+0x19d0:
fffff803`111e19d0 4053            push    rbx
fffff803`111e19d2 56              push    rsi

This callback routine checks a filename of a loaded image. If a module named chrome.exe, iexplore.exe, or firefox.exe is loaded, the routine modifies the module's import table, inserting a new IAT entry of their module SafeCheckCode64.dll to the top
in order to inject their dll into a major browser process.

The problem is a newly-added IAT entry is allocated outside the image-mapped area. Our function RVAToPtr apparently considers it as an invalid case.

0:000> lm m firefox
start             end                 module name
00007ff6`68500000 00007ff6`685aa000   firefox  C (private pdb symbols)  d:\symbols\firefox.pdb\1841D16ABEAE93BF4C4C44205044422E1\firefox.pdb

0:000> !dh firefox
...snip...
   5A96F [     D31] address [size] of Export Directory
   C0000 [     17C] address [size] of Import Directory   <<<< Outside the image!!
   6B000 [   3DB20] address [size] of Resource Directory
   64000 [    2508] address [size] of Exception Directory
       0 [       0] address [size] of Security Directory
   A9000 [     31C] address [size] of Base Relocation Directory
   59FB2 [      1C] address [size] of Debug Directory
       0 [       0] address [size] of Description Directory
       0 [       0] address [size] of Special Directory
   57078 [      28] address [size] of Thread Storage Directory
   56F60 [     108] address [size] of Load Configuration Directory
       0 [       0] address [size] of Bound Import Directory
   5C320 [     B18] address [size] of Import Address Table Directory
   5A590 [      E0] address [size] of Delay Import Directory
       0 [       0] address [size] of COR20 Header Directory
       0 [       0] address [size] of Reserved Directory
...snip...

0:000> !imp firefox
   0 00007ff6`685c0000 C:\Program Files (x86)\IObit\IObit Malware Fighter\SafeCheckCode64.dll  <<<< Dynamically-added!!
   1 00007ff6`685c0014 mozglue.dll
   2 00007ff6`685c0028 ADVAPI32.dll
   3 00007ff6`685c003c ntdll.dll
   4 00007ff6`685c0050 MSVCP140.dll
   5 00007ff6`685c0064 KERNEL32.dll
   6 00007ff6`685c0078 VCRUNTIME140.dll
   7 00007ff6`685c008c api-ms-win-crt-stdio-l1-1-0.dll
   8 00007ff6`685c00a0 api-ms-win-crt-environment-l1-1-0.dll
   9 00007ff6`685c00b4 api-ms-win-crt-runtime-l1-1-0.dll
  10 00007ff6`685c00c8 api-ms-win-crt-math-l1-1-0.dll
  11 00007ff6`685c00dc api-ms-win-crt-time-l1-1-0.dll
  12 00007ff6`685c00f0 api-ms-win-crt-convert-l1-1-0.dll
  13 00007ff6`685c0104 api-ms-win-crt-string-l1-1-0.dll
  14 00007ff6`685c0118 api-ms-win-crt-filesystem-l1-1-0.dll
  15 00007ff6`685c012c api-ms-win-crt-utility-l1-1-0.dll
  16 00007ff6`685c0140 api-ms-win-crt-locale-l1-1-0.dll
  17 00007ff6`685c0154 api-ms-win-crt-heap-l1-1-0.dll

A fix would be to loosen our check in a particular condition, probably when SafeCheckCode64.dll is already loaded in the process.

Comment 1

[(No longer employed by Mozilla) Aaron Klotz]

We have seen this before with other products and use RestoreImportDirectory to preemptively fix the IAT. Are we calling RestoreImportDirectory too late, or is it otherwise ineffective in this case?

Comment 2

[Toshihito Kikuchi [:toshi]]

(In reply to Aaron Klotz [:aklotz] from comment #1)

We have seen this before with other products and use RestoreImportDirectory to preemptively fix the IAT. Are we calling RestoreImportDirectory too late, or is it otherwise ineffective in this case?

Interesting! We're almost there. RestoreImportDirectory nicely restores the orignal IAT, but it's only for a child process. Since IAT of the launcher process is still kept modified, GetIATThunksForModule("ntdll.dll") after RestoreImportDirectory fails.

I'm not 100% sure, but I don't think it's safe (or even possible) to restore IAT in the launcher process as well. In this case the AV software does not modify IAT thunk of "ntdll.dll", so we can just continue to run InitializeDllBlocklistOOP if the thunk is located outside the image. What do you think?

Comment 3

[(No longer employed by Mozilla) Aaron Klotz]

(In reply to Toshihito Kikuchi [:toshi] from comment #2)

I'm not 100% sure, but I don't think it's safe (or even possible) to restore IAT in the launcher process as well. In this case the AV software does not modify IAT thunk of "ntdll.dll", so we can just continue to run InitializeDllBlocklistOOP if the thunk is located outside the image. What do you think?

Oh, I see, so the tampering is interfering with our ability to graft our own IAT. I guess we can try this, but I am hesitant to always bypass those checks. I think the best thing to do would be to add flags to GetIATThunksForModule that allow for skipping the bounds checks, so that the caller needs to opt in.

Comment 4

[Toshihito Kikuchi [:toshi]]

Attachment: Bug 1587539 - Skip bounds check when getting IAT if Import Directory is tampered. r=aklotz

Some applications tamper Import Directory entry of a loaded executable image
to pretend static dependency on their module. We have RestoreImportDirectory
to revert it in the browser process. If tampering happened in the launcher
process, however, we failed to get an IAT thunk address via GetIATThunksForModule
because it could be located outside the mapped image.

With this patch, we skip bounds check in GetIATThunksForModule if we detect
tampering in the launcher process. We can proceed safely because it's expected
that Import Directory is still valid though it's located outside.

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

The priority flag is not set for this bug.
:aklotz, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/848a87a2c19b
Skip bounds check when getting IAT if Import Directory is tampered. r=aklotz

Comment 7

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/848a87a2c19b