Assertion failure: compartment mismatched, at js/src/jscntxtinlines.h:542

missed macaw, we'll look at triaging into possible chemspills when we know the cause and fix.

What build does this happen in? mozilla-central, Firefox 4? regression between 4 and 4.0.1pre?

I can reproduce this in mozilla-central and Firefox 4, and this is not a regression between 4 and 4.0.1pre.

mrbkap: is this likely an sg:high same-origin violation, or an sg:critical worm-into-chrome problem? (or neither?) Minus for mozilla-2.0 for now, but if we have a ff5 fix in hand and do a chemspill we can pick it up then.

Blake, can you have a look here?

Blake says this bug in and of itself is not exploitable, but he's got a fix in the works that will fix this bug and also bug 650275.

Depends on Bug 650275 - Sheila commented in that bug asking for status.

Comment on attachment 531794 [details] [diff] [review] Fix Review of attachment 531794 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jswrapper.cpp @@ +375,5 @@ > + if (!context->stack.pushDummyFrame(context, *scopeChain, &frame)) > + return false; > + > + if (context->isExceptionPending()) > + context->wrapPendingException(); As discussed, I think this can be dropped since the compartment isn't changing.

(That is, the wrapPendingException, not the first two lines that splinter included. We need a way to control how much splinter quotes.)

Luke, Blake, what's the risk/reward trade-off here for potentially taking into Firefox 5?

Asa, this fixes a few security bugs, and adds belts-and-braces for other security bugs that we fixed in another way. It also maintains some invariants that were otherwise not well maintained in the face of malicious scripts. On the other hand, this isn't the safest patch, as it does add a couple of new cases that didn't exist before (and required at least one fix, in bug 656460). I'd say it's worth it, overall.

cdleary-bot mozilla-central merge info: http://hg.mozilla.org/mozilla-central/rev/ab172b4ca00d

This caused bug 659207. I would expect that to have a good chance of causing this to not build if pushed for fx5 as-is.

Comment on attachment 531794 [details] [diff] [review] Fix I think this should go into Firefox 5. It hasn't had fallout except for bug 659207 and that's a minor change to the patch.

Blake, is the patch at bug 650273 strictly necessary if we agree to take the patch here? Or is that something "only" of concern for our developers?

I don't think "'only' of concern for our developers is a valid phrase. If all you care about is cutting down on the number of patches pushed to Aurora, then I'm happy to fold the patches together. I don't want to split hairs over a compile fix with no effect on actual code. There is no advantage to taking this patch without the compile fix in bug 659207.

(In reply to comment #20) > I don't think "'only' of concern for our developers is a valid phrase. If > all you care about is cutting down on the number of patches pushed to > Aurora, then I'm happy to fold the patches together. I don't want to split > hairs over a compile fix with no effect on actual code. There is no > advantage to taking this patch without the compile fix in bug 659207. I'm not interested in numbers of patches at all. What I'm trying to figure out here is what part of these three issues, this bug, bug 650275 and bug 659207 are necessary to solve the security issue that I presumed was a concern for our users. That's why release drivers are tracking these bugs.

Well, except that release drivers are pretty explicitly not tracking bug 659207. For the record: This bug (bug 650273) requires bug 650275 (i.e. you can't land this bug without that one). bug 650275 only has an effect with this bug checked in (the problem that it fixes doesn't exist without the patch in this bug). bug 659207 fixes bz's build bustage as fallout from the patch here.

Per further discussion with mrbkap etc we've decided that it's too late to take this for 5, but this landed in time for 6 already, so it'll be fixed there.

Comment on attachment 531794 [details] [diff] [review] Fix removing approval since this is no longer intended for beta for 5.

I took some ideas from the testcase and added them to my DOM fuzzer. Thanks moz_bug_r_a4 and mrbkap. (7ea4f291dde2, 68b6a7a21708, 8958f6b556c7, 10570e474f13)