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?
Hey Aaron, what's the priority on this?
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.
I won't have time this week but should be able to take this later.
Assignee: nobody → davidp99
Priority: -- → P3
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 firstname.lastname@example.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/6147dace3d0e Use static nsWindowsDllInterceptor in plugin's HookProtectedMode r=aklotz
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox61: affected → fixed
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?
status-firefox59: --- → unaffected
status-firefox-esr52: --- → unaffected
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+
status-firefox60: affected → fixed
Setting qe-verify- based on David's assessment on manual testing needs (Comment 8).
You need to log in before you can comment on or make changes to this bug.