Assertion failure: IsObjectValueInCompartment(value, compartment())

VERIFIED FIXED in Firefox 33



JavaScript Engine
4 years ago
2 years ago


(Reporter: Tomcat, Assigned: jimb)


(Blocks: 1 bug, {assertion, sec-moderate})

assertion, sec-moderate
Bug Flags:
qe-verify +

Firefox Tracking Flags

(firefox33 verified, firefox-esr24 wontfix, firefox-esr31 wontfix)


(Whiteboard: [adv-main33+], URL)


(2 attachments)



4 years ago
Created attachment 8432435 [details]
bughunter stack

found via bughunter on

loading this site / or reloading cause

Assertion failure: IsObjectValueInCompartment(value, compartment()), at c:\work\mozilla\builds\nightly\mozilla\js\src\vm/ObjectImpl.h:731
(gdb) frame 0
#0  js::ObjectImpl::setSlot (this=0x133a760c0, slot=1, value=@0x7fff5fbfbbd8) at ObjectImpl.h:731
731             MOZ_ASSERT(IsObjectValueInCompartment(value, compartment()));
(gdb) p ((nsGlobalWindow*)js::GetReservedSlot(this->compartment()->global_.value, 0).toPrivate())->mDoc.mRawPtr->mDocumentURI.mRawPtr->mSpec
$23 = {
  <nsACString_internal> = {
    mData = 0x124989b88 "", 
    mLength = 46, 
    mFlags = 5
  }, <No data fields>}
(gdb) p ((nsGlobalWindow*)js::GetReservedSlot(value.toObjectOrNull()->compartment()->global_.value, 0).toPrivate())->mDoc.mRawPtr->mDocumentURI.mRawPtr->mSpec
$24 = {
  <nsACString_internal> = {
    mData = 0x124989b88 "", 
    mLength = 46, 
    mFlags = 5
  }, <No data fields>}
(gdb) p this->compartment()
$25 = (class JSCompartment *) 0x12de7f000
(gdb) p value.toObjectOrNull()->compartment()
$26 = (class JSCompartment *) 0x11b614800

This is the source element stuff for scripts; not a JIT issue afaict.  Jim, why aren't we wrapping that into the right compartment?
Component: JavaScript Engine: JIT → JavaScript Engine
Flags: needinfo?(jimb)
Keywords: sec-high

Comment 3

4 years ago
We're certainly supposed to be wrapping these things. Looking into it.

Comment 4

4 years ago
I am not able to reproduce this crash. But as far as I can tell, the owning element is supposed to be wrapped by this code:

  // XXXbz this is super-fragile, because the code that _uses_ the
  // JS::CompileOptions is nowhere near us, but we have to coordinate
  // compartments with it... and in particular, it will compile in the
  // compartment of aScopeChain, so we want to wrap into that compartment as
  // well.
  JSAutoCompartment ac(cx, aScopeChain);
  if (NS_SUCCEEDED(nsContentUtils::WrapNative(cx, aRequest->mElement,
                                              /* aAllowWrapping = */ true))) {

The crash is in an off-main-thread compilation. Is it possible that the current compartment of the JSContext passed to JS::FinishOffThreadScript might be different from the compartment of the JSContext passed to JS::CompileOffThread?
Flags: needinfo?(jimb)

Comment 5

4 years ago
It would be possible to change the rules for CompileOptions: it could be SpiderMonkey's responsibility to properly wrap the element. Then, the above "super-fragile" code could be deleted.

Comment 6

4 years ago
Created attachment 8447508 [details] [diff] [review]
In off-thread compilation, rewrap the compilation options that might need it before saving them on the ScriptSourceObject.

I can't reproduce the crash myself, but this patch addresses my best guess as to the problem. If the try push looks clean, please see if the patch fixes the crash.

If it does fix the crash, then the patch should be expanded to cover on-thread compilation as well.
I can't reproduce the crash anymore either.  Either the site or something in our code changed...

Comment 8

4 years ago
I don't think it's actually sec-critical. It's a compartment violate, but the only thing that ever uses the field that would have the bad cross-compartment reference is Debugger; if the user never opens the devtools, this bug can't affect them.

Even if they do, Debugger will immediately wrap the bad cross-compartment edge in a Debugger.Object, which should be pretty safe. The devtools will just see a surprising reference.

Comment 9

4 years ago
I've filed bug 1031636 to simplify this code. Perhaps the nsScriptLoader.cpp snarl mentioned above can come out if it lands.
Keywords: sec-high → sec-moderate

Comment 10

4 years ago
Now that I've landed bug 1031636, I would expect this not to occur any more. It's also no longer reproducible. Closing.
Last Resolved: 4 years ago
Resolution: --- → FIXED
Bob Clary asked me to change this back to FIXED in email, so I will do that.  My interpretation of FIXED was that a fix was in the bug itself, but I guess that does not match the description.
Assignee: nobody → jimb
status-firefox33: --- → fixed
Target Milestone: --- → mozilla33
status-firefox-esr24: --- → wontfix
status-firefox-esr31: --- → wontfix
Whiteboard: [adv-main33+]
Using URL in bug description:
Confirmed assert on Fx32, 2014-06-05.
Verified fixed on Fx33, 2014-10-06.
status-firefox33: fixed → verified
Flags: qe-verify+


3 years ago
Group: core-security → core-security-release
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.