Closed Bug 1300083 (CVE-2016-9072) Opened 3 years ago Closed 3 years ago

64-bit NPAPI sandbox isn't enabled on fresh profile

Categories

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

47 Branch
x86_64
Windows
defect

Tracking

()

VERIFIED FIXED
mozilla52
Tracking Status
firefox48 --- wontfix
firefox49 --- wontfix
firefox-esr45 --- unaffected
firefox50 + verified
firefox51 --- verified
firefox52 --- verified

People

(Reporter: bobowen, Assigned: bobowen)

References

Details

(Keywords: csectype-priv-escalation, regression, sec-high, Whiteboard: [post-critsmash-triage][adv-main50+] sbwn1)

Attachments

(1 file)

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.
The other constructor seems to be used for copying.
Attachment #8787605 - Flags: review?(jmathies)
Attachment #8787605 - Flags: review?(jmathies) → review+
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.
Attachment #8787605 - Flags: sec-approval?
Blocks: 1246574
Group: core-security → dom-core-security
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.
Keywords: sec-moderatesec-high
This can land on September 20, two weeks into the new cycle. It is too late for the 49 cycle.

sec-approval+ for then.
Whiteboard: [checkin on 9/20]
Attachment #8787605 - Flags: sec-approval? → sec-approval+
Whiteboard: [checkin on 9/20] → [checkin on 9/20] sbwc1
Whiteboard: [checkin on 9/20] sbwc1 → [checkin on 9/20] sbwn1
Priority: -- → P1
Is there any way to test this in automation?
Flags: needinfo?(bobowen.code)
Flags: in-testsuite?
(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.
Flags: needinfo?(bobowen.code)
(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?
Flags: needinfo?(abillings)
(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.
Flags: needinfo?(abillings) → needinfo?(rforbes)
[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 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
Flags: needinfo?(rforbes)
Attachment #8787605 - Flags: approval-mozilla-beta?
Attachment #8787605 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/41972bd40b59
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Comment on attachment 8787605 [details] [diff] [review]
Initialize Plugin Tags correctly

Sec-high, Aurora51+, Beta50+
Attachment #8787605 - Flags: approval-mozilla-beta?
Attachment #8787605 - Flags: approval-mozilla-beta+
Attachment #8787605 - Flags: approval-mozilla-aurora?
Attachment #8787605 - Flags: approval-mozilla-aurora+
Group: dom-core-security → core-security-release
Flags: qe-verify+
QA Contact: mwobensmith
Whiteboard: sbwn1 → [post-critsmash-triage] sbwn1
Whiteboard: [post-critsmash-triage] sbwn1 → [post-critsmash-triage][adv-main50+] sbwn1
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.
Alias: CVE-2016-9072
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.