The default bug view has changed. See this FAQ.
Bug 871368 (CVE-2013-1710)

Arbitrary code execution using crypto.generateCRMFRequest

RESOLVED FIXED in Firefox 23, Firefox OS v1.1hd

Status

()

Core
Security
RESOLVED FIXED
4 years ago
2 years ago

People

(Reporter: moz_bug_r_a4, Assigned: bholley)

Tracking

({csectype-priv-escalation, sec-critical})

unspecified
mozilla24
x86
Windows XP
csectype-priv-escalation, sec-critical
Points:
---

Firefox Tracking Flags

(firefox21 wontfix, firefox22+ wontfix, firefox23+ verified, firefox24+ verified, firefox-esr1723+ verified, b2g18 fixed, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 wontfix, b2g-v1.1hd fixed)

Details

(Whiteboard: [adv-main23+][adv-esr1708+][fixed by bug 871301, but keep the dep private])

Attachments

(1 attachment)

(Reporter)

Description

4 years ago
nsCrypto::GenerateCRMFRequest uses JS_GetGlobalObject, so a callback script can be executed with a global object that is unrelated to the caller.  This allows for a XSS attack or arbitrary code execution.

Bug 871301 seems to fix this.
(Reporter)

Comment 1

4 years ago
Created attachment 748621 [details]
testcase 1 - XSS

This triggers an assertion failure in debug builds.

This tries to get cookies for blog.mozilla.org.
(Reporter)

Comment 2

4 years ago
Created attachment 748622 [details]
testcase 2 - Arbitrary code execution

This triggers an assertion failure in debug builds.
Sigh. That bug is thankfully pretty innocuous-looking, so let's not mark the dependency. Let's get that landed.
Whiteboard: [dupe of bug 871301, but keep the dep private]
Whiteboard: [dupe of bug 871301, but keep the dep private] → [fixed by bug 871301, but keep the dep private]
Assignee: nobody → bobbyholley+bmo
Keywords: sec-critical
dchan, this API seems super old and not well-understood, but is nonetheless web exposed. Can you make sure we do some kind of cursory audit of this stuff sometime?
Flags: needinfo?(dchan+bugzilla)
I realized that the cx pushing in that bug is also going to cause a compartment mismatch with the current patch, because it pushes _after_ entering a compartment. I fixed it up in a hopefully-nonchalant fashion.
(also note that we appear to have zero test coverage of this function apart from one crashtest :-( )

Comment 7

4 years ago
Assertion failure: cx->compartment->principals == options.principals, at ../../../js/src/jsapi.cpp:5641

Comment 8

4 years ago
cf bug 327126
(In reply to Jesse Ruderman from comment #7)
> Assertion failure: cx->compartment->principals == options.principals, at
> ../../../js/src/jsapi.cpp:5641

Jesse, is this the assertion that we hit before bug 871301 landed (which I would expect)? Or does it happen on trunk?

Comment 10

4 years ago
That's on mozilla-central, which your fix hasn't hit yet.

Comment 11

4 years ago
I made some additions to my DOM fuzzer to help find bugs like this (a6e1e4bf9951, d2a2418aeaac).
bug 871301 is now fixed on m-c.

Matt: another bug to try in Fx22 and ESR-17 to see if we need to back-port fixes.
Status: NEW → RESOLVED
Last Resolved: 4 years ago
status-firefox23: --- → affected
status-firefox24: --- → fixed
tracking-firefox23: --- → +
tracking-firefox24: --- → +
Flags: needinfo?(mwobensmith)
Keywords: csec-priv-escalation
Resolution: --- → FIXED
We should uplift bug 871368 to all branches relatively close to the next branch. Al, when shall we do it?
Flags: needinfo?(abillings)
Crashes FF22, but does not seem to crash FF17.0.6esr.
status-firefox22: --- → affected
status-firefox-esr17: --- → unaffected
Flags: needinfo?(mwobensmith)
?

How do we upload a bug with no patch? Do you mean uplift bug 871301?
status-firefox22: affected → ---
status-firefox-esr17: unaffected → ---
Flags: needinfo?(abillings)
status-firefox22: --- → affected
status-firefox-esr17: --- → unaffected
(In reply to Al Billings [:abillings] from comment #15)
> ?
> 
> How do we upload a bug with no patch? Do you mean uplift bug 871301?

Yes.
Flags: needinfo?(abillings)
(Reporter)

Comment 17

4 years ago
testcase 1 no longer works because now blog.mozilla.org sends X-Frame-Options: SAMEORIGIN.  I'll attach a new testcase.
(Reporter)

Comment 18

4 years ago
Created attachment 752019 [details]
testcase 3 - XSS

This tries to get cookies for www.mozilla.org.
(Reporter)

Comment 19

4 years ago
I can reproduce testcase 2 (a Components.stack alert dialog appears) and testcase 3 (a XSS alert dialog appears) on fx23, 22, 21 and 17.  (I've tested only opt builds.)
status-firefox21: --- → affected
status-firefox-esr17: unaffected → affected
I want to loop in Release Management and see when they want to take this.
status-firefox21: affected → wontfix
Flags: needinfo?(abillings)
Marking B2G as affected unless someone wants to correct me.
status-b2g18: --- → affected
tracking-firefox22: --- → +
tracking-firefox-esr17: --- → +
Is bug 871301 something we'd still like to take into FF23? If so, please nominate for Aurora approval. It's too late in Beta to take a change of this nature.
status-firefox22: affected → wontfix
Created attachment 761051 [details]
Placeholder for bug 871301 patch

We probably need sec-approval.

[Security approval request comment]
How easily could an exploit be constructed based on the patch?

Somewhat easily. If someone looks at the function we're patching, it's pretty obvious that the API evaluates arbitrary strings, which is a pretty strong hint at how to start exploiting it.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?

No.

Which older supported branches are affected by this flaw?

The last decade. :-(

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

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?

Don't have backports, but they should be relatively straightforward.

How likely is this patch to cause regressions; how much testing does it need?

We're changing behavior in certain cases, but this is a mozilla-proprietary API. I'm more concerned about exploits than I am about regressions.
Attachment #761051 - Flags: sec-approval?
Also, it really sucks that this fell through the cracks for this cycle. I think the ideal time to land it would have been a month ago. :-( Where did we slip up here?
Comment on attachment 761051 [details]
Placeholder for bug 871301 patch

Sec-approval+ for trunk.

As to how it was missed...well, it wasn't really. It wasn't approved for branches, which is the problem here, and nothing got a sec-approval request for trunk. We get a lot of sec issues that we have to ping people on directly to get traction on patches a lot of the time.
Attachment #761051 - Flags: sec-approval? → sec-approval+
(In reply to Al Billings [:abillings] from comment #25)
> Comment on attachment 761051 [details]
> Sec-approval+ for trunk.

It's already on trunk. The whole question of this bug since comment 13 is when we check it in on branches, thereby revealing that it's a security issue.

> As to how it was missed...well, it wasn't really. It wasn't approved for
> branches, which is the problem here, and nothing got a sec-approval request
> for trunk. We get a lot of sec issues that we have to ping people on
> directly to get traction on patches a lot of the time.

It seems like comment 20 sought to answer the question of "when do we want to check this in on branches." I'm wondering what caused that to go unanswered for so many weeks, and if we can build a more robust mechanism for handling these things in the future.
Flags: needinfo?(abillings)
I don't get to answer that question for branches. That requires release management input. From now on, I'm going to needinfo? them or just have people mark the patches ? for approval for beta and/or aurora. I gather that these show up in queries.
Flags: needinfo?(abillings)
(In reply to Bobby Holley (:bholley) from comment #26)
> It seems like comment 20 sought to answer the question of "when do we want
> to check this in on branches." I'm wondering what caused that to go
> unanswered for so many weeks, and if we can build a more robust mechanism
> for handling these things in the future.

You should generally get that question answered by just requesting approval for aurora/beta/esr, especially for bugs already tracked.
> (In reply to Bobby Holley (:bholley) from comment #26)
> You should generally get that question answered by just requesting approval
> for aurora/beta/esr, especially for bugs already tracked.

Ok.

(In reply to Al Billings [:abillings] from comment #27)
> I don't get to answer that question for branches. That requires release
> management input. From now on, I'm going to needinfo? them or just have
> people mark the patches ? for approval for beta and/or aurora. I gather that
> these show up in queries.

That sounds good to me.
(In reply to Bobby Holley (:bholley) from comment #29)
> That sounds good to me.

Though I guess my concerns is that the release drivers are considering things from a regression risk perspective, but not necessarily from a zero-day perspective. I guess I can ask for that analysis to be done explicitly in my request.

I'll add a reminder for myself to nominate this for all branches on June 26th.
Comment on attachment 761051 [details]
Placeholder for bug 871301 patch

Now that the merge has happened, I'm flagging this for branch uplift. Please let me know when we want to land this timing-wise.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Since forever.
User impact if declined: sec-critial
Testing completed (on m-c, etc.): Baked on m-c for many weeks.
Risk to taking this patch (and alternatives if risky): Low risk. No alternative.
String or IDL/UUID changes made by this patch: None.
Attachment #761051 - Flags: approval-mozilla-esr17?
Attachment #761051 - Flags: approval-mozilla-beta?
Attachment #761051 - Flags: approval-mozilla-b2g18?
Attachment #761051 - Flags: approval-mozilla-esr17?
Attachment #761051 - Flags: approval-mozilla-esr17+
Attachment #761051 - Flags: approval-mozilla-beta?
Attachment #761051 - Flags: approval-mozilla-beta+
Attachment #761051 - Flags: approval-mozilla-b2g18?
Attachment #761051 - Flags: approval-mozilla-b2g18+
tracking-firefox-esr17: + → 23+
Do we want to land this now? Or wait a bit into the cycle to decrease the attack window? There is currently no public-facing indication that the patch in bug 871301 fixes a security issue.
Flags: needinfo?(lsblakk)

Comment 33

4 years ago
Oh wow. Sorry Bobby. I think my bugzilla filters are incorrect and I didn't get the needinfo from this a month ago. I'll add the crypto stuff to my list and talk to Yvan about it.
Flags: needinfo?(dchan+bugzilla)
Target Milestone: --- → mozilla24
Bholley - let's put this in, via an obfuscated comment in the week of 7/22 - I will put this in my reminder list to check back on.
Flags: needinfo?(lsblakk)
I've written up a bogus approval request in bug 871301.
b2g18 is closed at the moment, but it's in my queue to land there once it reopens.

https://hg.mozilla.org/releases/mozilla-beta/rev/59c478137f1e
https://hg.mozilla.org/releases/mozilla-esr17/rev/206dba739a7b
status-b2g18-v1.0.0: --- → wontfix
status-b2g18-v1.0.1: --- → wontfix
status-b2g-v1.1hd: --- → affected
status-firefox23: affected → fixed
status-firefox-esr17: affected → fixed
https://hg.mozilla.org/releases/mozilla-b2g18/rev/ea8e6808127f
https://hg.mozilla.org/releases/mozilla-b2g18_v1_1_0_hd/rev/ea8e6808127f
status-b2g18: affected → fixed
status-b2g-v1.1hd: affected → fixed
Whiteboard: [fixed by bug 871301, but keep the dep private] → [adv-main23+][adv-esr1708+][fixed by bug 871301, but keep the dep private]
Alias: CVE-2013-1710
Hi Bogdan,

This is the real issue behind bug 871301. Please do any verification work here and keep the dependency secret. Do not comment further in bug 871301.
Thx Bobby for giving me access to this bug.

Mozilla/5.0 (Windows NT 5.1; rv:25.0) Gecko/20100101 Firefox/25.0
Mozilla/5.0 (Windows NT 5.1; rv:24.0) Gecko/20130805 Firefox/24.0
Mozilla/5.0 (Windows NT 5.1; rv:23.0) Gecko/20100101 Firefox/23.0
Mozilla/5.0 (Windows NT 5.1; rv:17.0) Gecko/20100101 Firefox/17.0

Verified as fixed on Firefox 23 RC (buildID: 20130730113002), Firefox 17.0.8esr (buildID: 20130729214632) latest Aurora (buildID: 20130805004006) and latest Nightly (buildID: 20130804030207).
status-firefox23: fixed → verified
status-firefox24: fixed → verified
status-firefox-esr17: fixed → verified
Blocks: 1055189
Group: core-security
You need to log in before you can comment on or make changes to this bug.