Closed Bug 650273 Opened 9 years ago Closed 9 years ago

"Assertion failure: compartment mismatched" with setTimeout

Categories

(Core :: Security, defect)

x86
Windows XP
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla6
Tracking Status
firefox5 - wontfix
firefox6 + fixed
blocking2.0 --- -
status2.0 --- wanted
status1.9.2 --- unaffected
status1.9.1 --- unaffected

People

(Reporter: moz_bug_r_a4, Assigned: mrbkap)

References

Details

(Keywords: regression, Whiteboard: [sg:nse] fixed-in-tracemonkey [this and 659207 need approval together if at all])

Attachments

(2 files, 2 obsolete files)

Assertion failure: compartment mismatched, at js/src/jscntxtinlines.h:542
blocking2.0: --- → ?
blocking-fx: --- → ?
missed macaw, we'll look at triaging into possible chemspills when we know the cause and fix.
blocking2.0: ? → ---
blocking-fx: ? → ---
What build does this happen in? mozilla-central, Firefox 4? regression between 4 and 4.0.1pre?
blocking2.0: --- → ?
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.
blocking2.0: ? → -
status2.0: --- → wanted
Blake, can you have a look here?
Assignee: nobody → mrbkap
Keywords: regression
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.
Blocks: 650275
Whiteboard: [sg:nse]
No longer blocks: 650275
Blocks: 650275
Depends on Bug 650275 - Sheila commented in that bug asking for status.
Attached patch wip (obsolete) — Splinter Review
Attached patch Fix (obsolete) — Splinter Review
Attachment #531503 - Attachment is obsolete: true
Attachment #531782 - Flags: review?(luke)
Attached patch FixSplinter Review
Attachment #531782 - Attachment is obsolete: true
Attachment #531782 - Flags: review?(luke)
Attachment #531794 - Flags: review?(luke)
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.
Attachment #531794 - Flags: review?(luke) → review+
(That is, the wrapPendingException, not the first two lines that splinter included.  We need a way to control how much splinter quotes.)
http://hg.mozilla.org/tracemonkey/rev/ab172b4ca00d
Whiteboard: [sg:nse] → [sg:nse] fixed-in-tracemonkey
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.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
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.
Attachment #531794 - Flags: approval-mozilla-beta?
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.
Whiteboard: [sg:nse] fixed-in-tracemonkey → [sg:nse] fixed-in-tracemonkey [this and 659207 need approval together if at all]
(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.
Attachment #531794 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
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.
Attachment #531794 - Flags: approval-mozilla-beta+
Target Milestone: --- → mozilla6
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)
Group: core-security
You need to log in before you can comment on or make changes to this bug.