1300083 - (CVE-2016-9072) 64-bit NPAPI sandbox isn't enabled on fresh profile

Status: VERIFIED FIXED

(Core :: Security: Process Sandboxing, defect, P1)

Description

[Bob Owen (:bobowen)]

There may be other things apart from a new profile that cause this.

I noticed this by chance and it possibly explains why people trying to reproduce bug 1241250 were having problems, if they were using fresh installs and profiles.

It was introduced by bug 1246574.

[Tracking Requested - why for this release]
Very simple fix to ensure sandbox is always in place by default.

Comment 1

[Bob Owen (:bobowen)]

Attachment: Initialize Plugin Tags correctly

The other constructor seems to be used for copying.

Comment 2

[Bob Owen (:bobowen)]

Comment on attachment 8787605 [details] [diff] [review]
Initialize Plugin Tags correctly

[Security approval request comment]
How easily could an exploit be constructed based on the patch?
This isn't an exploit as such, but it does mean that any protection that the sandbox gives from exploits is not there in certain circumstances.
From the patch, it is fairly obvious that this is to do sandboxing, but it is probably not too clear in what circumstances the initialisation doesn't take place.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
Check-in comment is deliberately vague, but given the previous answer that probably doesn't matter much.

Which older supported branches are affected by this flaw?
Windows 64-bit Fx47+

If not all supported branches, which bug introduced the flaw?
Bug 1246574.

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
Same patch applies cleanly up to release.

How likely is this patch to cause regressions; how much testing does it need?
The resulting setting of the sandbox level is the same as before bug 1246574 and the same as when an existing profile is used now, so regressions seem unlikely.

Comment 3

[Daniel Veditz [:dveditz]]

You could argue this should be "sec-high" because we assume plugins have known exploitable bugs in them, especially flash because we know there's a market for them.

Comment 4

[Al Billings [:abillings - ex-MoCo]]

This can land on September 20, two weeks into the new cycle. It is too late for the 49 cycle.

sec-approval+ for then.

Comment 5

[Ryan VanderMeulen [:RyanVM]]

Is there any way to test this in automation?

Comment 6

[Bob Owen (:bobowen)]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #5)
> Is there any way to test this in automation?

I'm not sure if we test with real plugins, so I guess we'd have to sandbox the test one and get it to attempt to write somewhere.

We'd also have to make sure that when the test ran it was the first time it had been started with the profile.

Comment 7

[Bob Owen (:bobowen)]

(In reply to Al Billings [:abillings] from comment #4)
> This can land on September 20, two weeks into the new cycle. It is too late
> for the 49 cycle.
> 
> sec-approval+ for then.

Given that this cycle got pushed back, I just wanted to check whether you wanted to move the check-in of this back a bit?

Comment 8

[Bob Owen (:bobowen)]

(In reply to Bob Owen (:bobowen) from comment #7)
> (In reply to Al Billings [:abillings] from comment #4)
> > This can land on September 20, two weeks into the new cycle. It is too late
> > for the 49 cycle.
> > 
> > sec-approval+ for then.
> 
> Given that this cycle got pushed back, I just wanted to check whether you
> wanted to move the check-in of this back a bit?

Redirecting this question to rforbes, as I saw in an email that Al is out.

I should point out that this issue is causing a problem with the website owners testing bug 1241250.
They keep thinking they've fixed it, because I think they are using new profiles each time.

The extra noise there might mean someone else spots the problem.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/41972bd40b59

Comment 10

[Chris Peterson [:cpeterson]]

[Tracking Requested - why for this release]:

We should uplift this one-line fix to Beta 50 because it is a sec-high regression. Fortunately, it only affects 64-bit Firefox users.

Comment 11

[Bob Owen (:bobowen)]

Comment on attachment 8787605 [details] [diff] [review]
Initialize Plugin Tags correctly

Looks like a decision was made on landing this, so rforbes -= ni;

Approval Request Comment
[Feature/regressing bug #]:
Bug 1246574.

[User impact if declined]:
The first time they use a new profile the 64-bit flash NPAPI process will not be sandboxed.

[Describe test coverage new/current, TreeHerder]:
Manually tested, good candidate for separate QE verify.

STR:
* Install 64-bit Firefox before fix.
* Create new profile and start Firefox.
* Browse to a site that uses flash.
* Use Microsoft Sysinternals Process Explorer to check integrity level of flash NPAPI plugin-container process - it will be medium and should be low. (I think you have to add the column to the main table, right click on column headings ... it's under the Process Image tab.)

[Risks and why]: 
Low - simple one line fix.

[String/UUID change made/needed]:
None

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/41972bd40b59

Comment 13

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8787605 [details] [diff] [review]
Initialize Plugin Tags correctly

Sec-high, Aurora51+, Beta50+

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/9d6a1ee63674
https://hg.mozilla.org/releases/mozilla-beta/rev/93dcc6faa48f

Comment 15

[Mihai Boldan, Desktop QA [:mboldan]]

I managed to reproduce this issue on Firefox 49.0, using the STR from Comment 11.

The issue is no longer reproducible (the integrity of the plugin-container is low) on Firefox 50.0, Firefox 51.0a2 (2016-11-07) (64-bit), Firefox 52.0a1 (2016-11-08) (64-bit).

The tests were performed under Windows 10x64.