Do not use restricting SIDs in NPAPI process except on Nightly

RESOLVED FIXED in Firefox 60

Status

()

defect
P1
normal
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: handyman, Assigned: handyman)

Tracking

unspecified
mozilla61
Unspecified
Windows
Points:
---

Firefox Tracking Flags

(firefox60 fixed, firefox61 fixed)

Details

Attachments

(1 attachment)

This is to undo the work in Bug 1426733 on all builds except nightly.  Right now, restricting SIDs are on in the NPAPI proc in nightly and beta and we are running into some crashes around audio device usage -- see bug 1449388.  We think that the crash is due to a regression in Flash that Adobe is fixing.  In the interim, we think we should hold the use of restricting SIDs (they _harden_ the sandbox).
(Assignee)

Comment 1

a year ago
Bob, you can get caught up on the history in comment 0.  It appears Adobe is on it but we need to do something in the interim since its causing crashes in beta so the plan is to uplift this.  So far, the audio crash is the only known fallout from that rest SIDs change.  We can always undo the undo if Adobe comes back with something in time.

I'm nearly done playing with this locally and so far it doesn't have any conflict with the latest plugin setup (meaning it doesn't conflict with the access token change -- which really isn't a surprise).

---

Tests: https://treeherder.mozilla.org/#/jobs?repo=try&revision=40b29602e02fcc45ea344de39a531e9a6f9fd1bf
Attachment #8964442 - Flags: review?(bobowencode)
Attachment #8964442 - Flags: review?(bobowencode) → review+
(Assignee)

Updated

a year ago
Keywords: checkin-needed

Comment 2

a year ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/5365c443117b
Remove restricting SIDs from NPAPI sandbox outside of nightly builds. r=bobowen
Keywords: checkin-needed

Comment 3

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/5365c443117b
Status: NEW → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
(Assignee)

Comment 4

a year ago
Comment on attachment 8964442 [details] [diff] [review]
Remove restricting SIDs from NPAPI sandbox outside of nightly builds

Approval Request Comment
[Feature/Bug causing the regression]:
bug 1426733 hardened the plugin sandbox, inspiring some crashes

[User impact if declined]:
We _believe_ Flash usage will cause occasional crashes when doing things like changing audio devices (e.g. switching to headphones).  We _know_ that Flash will not recognize such device changes without this patch.

[Is this code covered by automated tests?]:
Not really.  The sandbox is created in at least one test but this is really relevant to Flash code, which we don't test.

[Has the fix been verified in Nightly?]:
The patch explicitly #ifdefs out any behavioral changes to nightly.  This is only intended to affect beta/release.

[Needs manual test from QE? If yes, steps to reproduce]: 
No.  The issue, however, can be verified by running any page with Flash content that makes sounds (like videos) and plugging/unplugging headphones.

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

[Is the change risky?]:
No.  

[Why is the change risky/not risky?]:
This lowers the power of the sandbox to a setting closer to where it is in release now.  A weaker sandbox shouldn't break anything.

[String changes made/needed]:
None
Attachment #8964442 - Flags: approval-mozilla-beta?
Comment on attachment 8964442 [details] [diff] [review]
Remove restricting SIDs from NPAPI sandbox outside of nightly builds

weaken flash plugin sandbox, approved for 60.0b11 :/
Attachment #8964442 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Blocks: 1449388
See Also: 1449388
You need to log in before you can comment on or make changes to this bug.