This is to fix the NPAPI part of bug 1119878, see bug 1119878 comment 8. It appears that the process handles are not actually needed and that we can take a similar approach to bug 1117140.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f1b2e6919479 Just thought, after pushing this to try, whether there is a danger that people could watch try pushes and glean information about secure bugs from them. I don't think it's a huge problem for this bug, since the only place we're properly using the sandbox is GMP, where this is already fixed and we've already gone public on that bug anyway.

(In reply to Bob Owen (:bobowen) from comment #1) > Just thought, after pushing this to try, whether there is a danger that > people could watch try pushes and glean information about secure bugs from > them. Yes, that's a problem in many cases. Unfortunately we don't have any kind of "shadow try" or plans for it.

I'd forgotten that ProcessHandle is just an int on posix: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0f9b0922c929

We don't normally push security bugs to try if they haven't received sec-approval+ (and are the kinds of bugs that require approval to go in). Pushing to try is, indeed, the same as just publicly checking in.

(In reply to Al Billings [:abillings] from comment #4) > We don't normally push security bugs to try if they haven't received > sec-approval+ (and are the kinds of bugs that require approval to go in). > Pushing to try is, indeed, the same as just publicly checking in. Yeah, sorry, I should have thought.

(In reply to Bob Owen (:bobowen) from comment #5) > Yeah, sorry, I should have thought. It's ok. Apparently, I'm not quite correct so now the security group is having a discussion of what we *should* do here. So, no fault, no foul!

(In reply to Bob Owen (:bobowen) from comment #3) > I'd forgotten that ProcessHandle is just an int on posix: > https://treeherder.mozilla.org/#/jobs?repo=try&revision=0f9b0922c929 Looks like this is not quite so simple. I'm getting a leaked gfxASurface, which may well be down to OtherProcess() being null on this line: https://dxr.mozilla.org/mozilla-central/source/dom/plugins/ipc/PluginInstanceChild.cpp#3400

[Tracking Requested - why for this release]: Are we going to want this on 36?

No.

I'm on PTO from 12-19 Feb, so deassigning for now.

If we're going to take this in 37, we're going to need an owner. Bob is back in a couple of days. Perhaps he wants to pick this back up then.

(In reply to Lawrence Mandel [:lmandel] (use needinfo) from comment #11) > If we're going to take this in 37, we're going to need an owner. > > Bob is back in a couple of days. Perhaps he wants to pick this back up then. The problem seems to be in the way that we are sharing memory to other processes. I don't really have any experience in this area, but I think we're going to need to find a solution for windows content sandboxing to get to low integrity as well, so I can start to look into it if no-one else is available.

If my fix works for bug 1119878, it'll fix this as well.

Does this actually affect 37, or does it only affect channels where we have e10s enabled?

(In reply to Liz Henry (:lizzard) from comment #14) > Does this actually affect 37, or does it only affect channels where we have > e10s enabled? It affects things if you have the NPAPI sandbox enabled for anything, this isn't turned on by default for anything at the moment. Given the strength of current policies we'd be able to set anyway, I'm not sure that this security flaw matters at the moment, as I think you'd be able to just open the process handle anyway. Either way round, if people are OK with my patches for bug 1119878, then they will fix things for this as well. At least as far as holding handles as a matter of course in the child process goes.

I assume at this point we're definitely not going to use this sandbox for 37, so I'll just mark it as disabled.

OK so based on what you're saying we can mark this disabled for 38 and 39. If that changes please switch it back to affected so we know it's an issue again.

Fix for bug 1119878 has landed, which fixes this for the NPAPI process (and any others) as well.

Post-CritSmash triage, setting flag for qe-verify-. https://securitywiki.mozilla.org/PostCritSmash