Closed Bug 1112709 Opened 9 years ago Closed 9 years ago

Flash Protected Mode Hook doesn't work on Windows 8/8.1: CreateFileW isn't hooking properly

Categories

(Core Graveyard :: Plug-ins, defect)

x86
Windows 8.1
defect
Not set
normal

Tracking

(firefox36 verified, firefox37 verified)

VERIFIED FIXED
mozilla37
Tracking Status
firefox36 --- verified
firefox37 --- verified

People

(Reporter: marcia, Assigned: m_kato)

References

Details

Attachments

(2 files)

QA testing of Bug 1108035 indicated there is an issue related to 8.1

https://etherpad.mozilla.org/FlashProtectedMode has the specifics from testing. Filing this bug to track the specific fix needed so that Windows 8.1 behaves properly.
Changing title.
Summary: Hook functions in plugin-container for Flash to configure Flash protected mode off → Issues with Window 8.1 and Flash protected mode
Blocks: 1108035
Summary: Issues with Window 8.1 and Flash protected mode → Flash Protected Mode Hook doesn't work on Windows 8/8.1: CreateFileW isn't hooking properly
It appears that WindowsDllInterceptor isn't working in this case. Details:

For the NopSpacePatcher:
`fn` is correct, and points to the following assembly:

_CreateFileA@28:
77457410 FF 25 48 03 4B 77    jmp         dword ptr ds:[774B0348h]  
77457416 CC                   int         3  
77457417 CC                   int         3  
77457418 CC                   int         3  
77457419 CC                   int         3  
7745741A CC                   int         3  
7745741B CC                   int         3  
_CreateFileW@28: this is the kernel32.dll _CreateFileW@28
7745741C FF 25 44 03 4B 77    jmp         dword ptr ds:[774B0344h]   `fn` points here
77457422 CC                   int         3  
77457423 CC                   int         3  
77457424 CC                   int         3  
77457425 CC                   int         3  
77457426 CC                   int         3  
77457427 CC                   int         3  

So the initial 5-byte nop loop succeeds, but the subsequent check for a MOV fails.

The indirection of *0x774b03ff ends us up at an entirely different _CreateFileW@28 function at 0x76DF23DF which is Kernelbase.dll.

So it seems to me that we can either 1) do some kind of version-specific kernel32/kernelbase magic in the client or 2) teach nsWindowsDllInterceptor about this redirection pattern and have it patch the real function, not the redirector.

Ehsan/m_kato/dmajor/aklotz: do you have thoughts about which of these makes more sense?
Flags: needinfo?(m_kato)
Flags: needinfo?(ehsan.akhgari)
Flags: needinfo?(dmajor)
Flags: needinfo?(aklotz)
The detours patcher can handle this case when the jmp is opcode 0x39 (near, relative), in which case it just modifies the target address of the jmp to point to the hook function. This case is a bit different since it's a far, absolute indirect jmp; I think we should add this case to the detour patcher's repertoire.
Flags: needinfo?(aklotz)
We should analyze FF /4 or JMP [disp32].  Also, we should investigate Win10 for this too.
Flags: needinfo?(m_kato)
I also think that we should extend the detour patches to understand this case.
Flags: needinfo?(ehsan.akhgari)
Assignee: nobody → m_kato
Attached patch fixSplinter Review
Comment on attachment 8539989 [details] [diff] [review]
fix

We need support jmp [disp32] opcode for x86.

Also, Flash's protected mode supports x86 binary only, so we don't need it for x64 flash.  And, to hook CreateFileW on x64 binary with Win8, we need fix of bug 1113546 at first.
Attachment #8539989 - Flags: review?(ehsan.akhgari)
Attachment #8539989 - Flags: review?(ehsan.akhgari) → review+
https://hg.mozilla.org/mozilla-central/rev/d78f8b811a39
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
Teaching nsWindowsDllInterceptor about this sounds good to me.
Flags: needinfo?(dmajor)
Blocks: 1119941
Comment on attachment 8539989 [details] [diff] [review]
fix

Approval Request Comment
[Feature/regressing bug #]: Bug 1108035 was ineffective on Windows 8/8.1 because of this bug.
[User impact if declined]: Bug 1119941 will not be effective on win8/8.1.
[Describe test coverage new/current, TBPL]: Has automated tests and has been in mozilla-central
[Risks and why]: Moderate-to-low risk: it's possible that this change could affect other functions which Firefox was already hooking, but we don't know of any and haven't seen ill effects.
[String/UUID change made/needed]: None
Attachment #8539989 - Flags: approval-mozilla-aurora?
Attachment #8539989 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
I finished testing the fix pushed for this issue on Nightly 37.0a1 (2015-01-11), using Windows 8.1 32-bit and Windows 8 32-bit, but I'm still seeing the bug.

Here's an overview:
(A) Windows 8.1 32-bit (e10s enabled)
  (A.1) dom.ipc.plugins.flash.disable-protected-mode = false (default)
    * 2 x "Adobe Flash Player 16.0 r0" processes active
    * 2 x "Plugin Container for Nightly" processes active
  (A.2) dom.ipc.plugins.flash.disable-protected-mode = true
    * 2 x "Adobe Flash Player 16.0 r0" processes active
    * 2 x "Plugin Container for Nightly" processes active

(B) Windows 8 32-bit (e10s enabled)
  (B.1) dom.ipc.plugins.flash.disable-protected-mode = false (default)
    * 2 x "Adobe Flash Player 16.0 r0" processes active
    * 2 x "Plugin Container for Nightly" processes active
  (B.2) dom.ipc.plugins.flash.disable-protected-mode = true
    * 2 x "Adobe Flash Player 16.0 r0" processes active
    * 2 x "Plugin Container for Nightly" processes active

Similar to Bug 1108035, the tests were performed on various websites with flash content, using Flash 16.0.0.235. Please note that the browser was restarted after each pref change, to make sure that the new value is applied successfully.

Makoto, any thoughts on this?
Flags: needinfo?(m_kato)
(In reply to Andrei Vaida, QA [:avaida] from comment #12)
> I finished testing the fix pushed for this issue on Nightly 37.0a1
> (2015-01-11), using Windows 8.1 32-bit and Windows 8 32-bit, but I'm still
> seeing the bug.
> 
> Here's an overview:
> (A) Windows 8.1 32-bit (e10s enabled)
>   (A.1) dom.ipc.plugins.flash.disable-protected-mode = false (default)
>     * 2 x "Adobe Flash Player 16.0 r0" processes active
>     * 2 x "Plugin Container for Nightly" processes active
>   (A.2) dom.ipc.plugins.flash.disable-protected-mode = true
>     * 2 x "Adobe Flash Player 16.0 r0" processes active
>     * 2 x "Plugin Container for Nightly" processes active
> 
> (B) Windows 8 32-bit (e10s enabled)
>   (B.1) dom.ipc.plugins.flash.disable-protected-mode = false (default)
>     * 2 x "Adobe Flash Player 16.0 r0" processes active
>     * 2 x "Plugin Container for Nightly" processes active
>   (B.2) dom.ipc.plugins.flash.disable-protected-mode = true
>     * 2 x "Adobe Flash Player 16.0 r0" processes active
>     * 2 x "Plugin Container for Nightly" processes active
> 
> Similar to Bug 1108035, the tests were performed on various websites with
> flash content, using Flash 16.0.0.235. Please note that the browser was
> restarted after each pref change, to make sure that the new value is applied
> successfully.
> 
> Makoto, any thoughts on this?

Although I test on Windows 7 SP1 with Nightly 2014-01-12, dom.ipc.plugins.flash.disable-protected-mode doesn't work.  Could you test on Windows 7 too?

Parent process doesn't seem to call SendDisableFlashProtectedMode.
Flags: needinfo?(m_kato)
I will continue as bug 1120747.  It doesn't depended on this.
Windows 8.1 issue is both bug 1120747 and the following.

1. Flash 16 call CreateFileA("...\mmc.cfg"), not CreateFileW
2. Windows 8.1 UP1 is the following.  So we cannot hook reading mmc.cfg 

0:009> u kernel32!CreateFileA
KERNEL32!CreateFileA:
76af8920 ff259403b576    jmp     dword ptr [KERNEL32!_imp__CreateFileA (76b50394)]

0:007> u poi(KERNEL32!_imp__CreateFileA)
KERNELBASE!CreateFileA:
74fd7190 8bff            mov     edi,edi
74fd7192 55              push    ebp
74fd7193 8bec            mov     ebp,esp
...

74fd71b9 ff75fc          push    dword ptr [ebp-4]
74fd71bc e8dfcdffff      call    KERNELBASE!CreateFileW (74fd3fa0)
74fd71c1 8bf0            mov     esi,eax

So we need support redirection from kenrel32 to kernelbase or should hook CreteFileA instead.
(In reply to Makoto Kato (:m_kato) from comment #14)
> Although I test on Windows 7 SP1 with Nightly 2014-01-12,
> dom.ipc.plugins.flash.disable-protected-mode doesn't work.  Could you test
> on Windows 7 too?

I'm seeing the same behavior on Windows 7 32-bit (SP1) as well, using Nightly 38.0a1 (2015-01-12) with e10s enabled:
 (A) dom.ipc.plugins.flash.disable-protected-mode = false (default)
    * 2 x "plugin-container.exe" processes active
    * 2 x "FlashPlayerPlugin_16_0_0_235.exe" processes active
 (B) dom.ipc.plugins.flash.disable-protected-mode = true
    * 2 x "plugin-container.exe" processes active
    * 2 x "FlashPlayerPlugin_16_0_0_235.exe" processes active

I used the following websites in my tests: 
 * http://www.albinoblacksheep.com/flash/wafflesloop
 * https://games.yahoo.com/
 * https://www.youtube.com/
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attached patch BUG1112709Splinter Review
I think your fix is better, but for the record here's a patch which hooks CreateFileA in addition to CreateFileW.
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #19)
> Created attachment 8549164 [details] [diff] [review]
> BUG1112709
> 
> I think your fix is better, but for the record here's a patch which hooks
> CreateFileA in addition to CreateFileW.


Your fix hooks both functions.  It is OK for Windows 8.1, but if on Windows 7, CreateFileA may call both hooked functions.

On Windows 7, kenrel32!CreateFileA will call kernel32!CreateFileW.
On Windows 8.1, kerenl32!CreateFileA will call kernelbase!CreateFileA, then it will call kenrelbase!CreateFileW.

Disassembling Windows 7's CreateFileA.

0:000> u kernel32!CreateFileA
kernel32!CreateFileA:
7652eb11 8bff            mov     edi,edi
...
7652eb3f ff75fc          push    dword ptr [ebp-4]
7652eb42 e80efeffff      call    kernel32!CreateFileWImplementation
...

0:000> u 7652e955
kernel32!CreateFileWImplementation:
7652e955 8bff            mov     edi,edi
...
7652e997 ff7508          push    dword ptr [ebp+8]
7652e99a e809000000      call    kernel32!CreateFileW (7652e9a8)
...
benjamin creates new fix for hooking CreateFileA, so redirection issue is moved to bug 1121829
To be clear, I don't want to take my patch, I just want to take bug 1121829. I'm going to mark this bug FIXED again since the patch here landed and got uplifted, and set tracking flags on your other one.
Status: REOPENED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → FIXED
Depends on: 1121829
Makoto, given that Bug 1120747 is now verified fixed and Windows 7 is no longer affected, is there anything else to be done by manual QA here?
Flags: needinfo?(m_kato)
(In reply to Andrei Vaida, QA [:avaida] from comment #23)
> Makoto, given that Bug 1120747 is now verified fixed and Windows 7 is no
> longer affected, is there anything else to be done by manual QA here?

I think you should verify protected mode issue on Windows 8.1 with bug 1123966.
Flags: needinfo?(m_kato)
This is verified fixed on Aurora 37.0a2 (2015-02-02) and Beta 36.0b5 (20150129200438), using Windows 7 32 bit.
Status: RESOLVED → VERIFIED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.