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.
Created attachment 748621 [details] testcase 1 - XSS This triggers an assertion failure in debug builds. This tries to get cookies for blog.mozilla.org.
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.
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?
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 :-( )
Assertion failure: cx->compartment->principals == options.principals, at ../../../js/src/jsapi.cpp:5641
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?
That's on mozilla-central, which your fix hasn't hit yet.
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.
We should uplift bug 871368 to all branches relatively close to the next branch. Al, when shall we do it?
Crashes FF22, but does not seem to crash FF17.0.6esr.
? How do we upload a bug with no patch? Do you mean uplift bug 871301?
(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.
testcase 1 no longer works because now blog.mozilla.org sends X-Frame-Options: SAMEORIGIN. I'll attach a new testcase.
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.)
I want to loop in Release Management and see when they want to take this.
Marking B2G as affected unless someone wants to correct me.
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.
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.
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.
(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.
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.
(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.
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.
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.
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.
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
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).