Interceptor in FunctionHook::HookProtectedMode should be persistent

RESOLVED FIXED in Firefox 60

Status

()

defect
P1
normal
RESOLVED FIXED
Last year
Last year

People

(Reporter: aklotz, Assigned: handyman)

Tracking

Trunk
mozilla61
Unspecified
Windows
Points:
---
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox59 unaffected, firefox60 fixed, firefox61 fixed)

Details

Attachments

(1 attachment)

The interceptor in FunctionHook::HookProtectedMode is allocated on the stack. When destroyed, that interceptor will fix up its trampolines such that the hooks are effectively inert.

That interceptor needs to live beyond the lifetime of HookProtectedMode's scope.
David, could you take a look at this please?
Flags: needinfo?(davidp99)
Hey Aaron, what's the priority on this?
Flags: needinfo?(aklotz)
This does break pref-based disabling of Protected Mode in 32-bit flash.

I don't know what the current state of Flash Sandboxing is on 32-bit Windows, but if we intend to override their sandbox by default, this should be high priority. Otherwise it's not too urgent.
Flags: needinfo?(aklotz)
I won't have time this week but should be able to take this later.
Assignee: nobody → davidp99
Flags: needinfo?(davidp99)
Priority: -- → P3
Priority: P3 → P1
There was already an nsWindowsDllInterceptor intended for this purpose -- this was essentially a cut-and-paste fail.
Attachment #8965044 - Flags: review?(aklotz)
Attachment #8965044 - Flags: review?(aklotz) → review+
Pushed by btara@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6147dace3d0e
Use static nsWindowsDllInterceptor in plugin's HookProtectedMode r=aklotz
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/6147dace3d0e
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Comment on attachment 8965044 [details] [diff] [review]
Use static nsWindowsDllInterceptor in plugin's HookProtectedMode

Approval Request Comment
[Feature/Bug causing the regression]:
bug 1382251 refactored the nsWindowsDllInterceptor usage and introduced this regression

[User impact if declined]:
The Adobe Protected Mode sandbox (the 32 bit sandbox for Flash) cannot be disabled in about:addons.

[Is this code covered by automated tests?]:
No

[Has the fix been verified in Nightly?]:
Yes

[Needs manual test from QE? If yes, steps to reproduce]: 
No

[List of other uplifts needed for the feature/fix]:
None

[Is the change risky?]:
No

[Why is the change risky/not risky?]:
This small change restores long-standing behavior to the way it existed before the refactor.

[String changes made/needed]:
None
Attachment #8965044 - Flags: approval-mozilla-beta?
Comment on attachment 8965044 [details] [diff] [review]
Use static nsWindowsDllInterceptor in plugin's HookProtectedMode

flash sandboxing fix, approved for 60.0b12
Attachment #8965044 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Setting qe-verify- based on David's assessment on manual testing needs (Comment 8).
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.