"Assertion failure: false (compartment mismatched)," or "Assertion failure: JSVAL_IS_OBJECT_OR_NULL_IMPL(JSVAL_TO_IMPL(v)),"

VERIFIED FIXED in mozilla17

Status

()

Core
JavaScript Engine
--
minor
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: gkw, Assigned: luke)

Tracking

(Blocks: 1 bug, {assertion, regression, testcase})

Trunk
mozilla17
x86
Windows 7
assertion, regression, testcase
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [shell only])

Attachments

(3 attachments)

(Reporter)

Description

5 years ago
s = newGlobal()
try {
  evalcx("\
	Object.defineProperty(this,\"i\",{enumerable:true,get:function(){t}});\
	for each(y in this)true\
  ", s)
} catch (e) {}
try {
  evalcx("\
	for(z=0,(7).watch(\"\",eval);;g){\
	  if(z=1){({t:function(){}})\
	}\
	", s)
} catch (e) {}
try {
  evalcx("\
	Object.defineProperty(this,\"g2\",{get:function(){return this}});\
	g2.y()\
  ", s)
} catch (e) {}

asserts js debug shell on m-c changeset 030feb4315fc without any CLI arguments at Assertion failure: false (compartment mismatched), while a variant asserts at Assertion failure: JSVAL_IS_OBJECT_OR_NULL_IMPL(JSVAL_TO_IMPL(v)) :


s = newGlobal()
try {
  evalcx("\
	Object.defineProperty(this,\"i\",{enumerable:true,get:function(){t}});\
	for each(y in this)true\
  ", s)
} catch (e) {}
try {
  evalcx("\
	for(z=0,(7).watch(\"\",eval);;g){\
	  if(z=1){({t:function(){}})\
	}\
	", s)
} catch (e) {}
try {
  evalcx("\
	Object.defineProperty(this,\"g2\",{get:function(){return this}});\
	g2.y(\"\")\
  ", s)
} catch (e) {}
(Reporter)

Updated

5 years ago
Group: core-security
Keywords: assertion
Summary: "Assertion failure: false (compartment mismatched)," or "Assertion failure: JSVAL_IS_OBJECT_OR_NULL_IMPL(JSVAL_TO_IMPL(v)), → "Assertion failure: false (compartment mismatched)," or "Assertion failure: JSVAL_IS_OBJECT_OR_NULL_IMPL(JSVAL_TO_IMPL(v)),"
(Reporter)

Comment 1

5 years ago
Locking s-s because compartment mismatched bugs are usually bad.
status-firefox17: --- → affected
(Reporter)

Updated

5 years ago
See Also: → bug 767817
(Reporter)

Comment 2

5 years ago
(not sure which one is correct)

Regression window of the first testcase in comment 0:

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   97470:68c396f305f4
user:        Luke Wagner
date:        Wed Jun 20 08:57:29 2012 -0700
summary:     Bug 755186 - rm JS_NewGlobalObject (r=jorendorff)

Regression window of the second testcase in comment 0:

The first bad revision is:
changeset:   101160:a91040f69ea3
user:        Steve Fink
date:        Mon Jul 23 13:37:31 2012 -0700
summary:     Bug 777219 - Prepare SpiderMonkey for a fully rooted API; r=bhackett
(Assignee)

Comment 3

5 years ago
Does this reproduce for you on anything other than 32-bit windows?  I can't repro on 64-bit debug linux.
By the way, it's really useful to have a stack trace along with bugs like this.
(Reporter)

Comment 5

5 years ago
Created attachment 648894 [details]
stack

Stack for "Assertion failure: false (compartment mismatched),"

Sorry for missing out the stack, also repros in m-c changeset a7fadfbad932
(Reporter)

Comment 6

5 years ago
Created attachment 648895 [details]
stack

Stack for "Assertion failure: JSVAL_IS_OBJECT_OR_NULL_IMPL(JSVAL_TO_IMPL(v)),"
(Reporter)

Comment 7

5 years ago
(In reply to Luke Wagner [:luke] from comment #3)
> Does this reproduce for you on anything other than 32-bit windows?  I can't
> repro on 64-bit debug linux.

I don't think I can check this till next week.
(Assignee)

Comment 8

5 years ago
These both seem related to the Help() shell function...
(Assignee)

Comment 9

5 years ago
Oh, heh:
 - for comment 5, I suspect the problem is that JS_GetGlobalObject(cx) can be in a different compartment than cx, so we need to AutoEnterCompartment
 - for comment 7, I think we need to check JSVAL_IS_PRIMITIVE before calling JSVAL_TO_OBJECT
I'm going to guess at sec-moderate because comment 8 seems to say this specific testcase isn't a problem in release builds, but comment 9 is saying there might be a more generic issue underlying it.
Keywords: sec-moderate
(Assignee)

Comment 11

5 years ago
Oh, no, comment 9 is just commenting on two bugs in the shell function Help().
I'm going to mark it sec-critical then due to the compartment mismatch.
Keywords: sec-moderate → sec-critical
(Assignee)

Comment 13

5 years ago
The compartment mismatch is caused by the shell function, so I don't think it's s-s.  I'll just write a patch and see if I'm right; I was just eye-balling it in comment 9.
(Assignee)

Comment 14

5 years ago
Created attachment 650669 [details] [diff] [review]
maybe fix

I can't repro the crash so I can't verify the assert, but this patch fixes the two flagrant bugs.  Gary: can you verify?
Attachment #650669 - Flags: feedback?(gary)
(Reporter)

Comment 15

5 years ago
Comment on attachment 650669 [details] [diff] [review]
maybe fix

I verify that the 2 asserts are indeed fixed by this patch, thanks Luke!
Attachment #650669 - Flags: feedback?(gary) → feedback+
(Assignee)

Comment 16

5 years ago
Comment on attachment 650669 [details] [diff] [review]
maybe fix

(Assume I add the test-cases)
Attachment #650669 - Flags: review?(wmccloskey)
(Assignee)

Comment 17

5 years ago
comment 15 confirms comment 13, so removing s-s.
Group: core-security
Keywords: sec-critical
Whiteboard: [shell only]
Attachment #650669 - Flags: review?(wmccloskey) → review+
(Assignee)

Comment 18

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/bb4c98f406c0
Severity: critical → minor
Target Milestone: --- → mozilla17
https://hg.mozilla.org/mozilla-central/rev/bb4c98f406c0
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED

Updated

5 years ago
status-firefox17: affected → ---
(Reporter)

Comment 20

5 years ago
Testcases have been added to the testsuite -> VERIFIED.
Status: RESOLVED → VERIFIED
Flags: in-testsuite+
(Reporter)

Updated

5 years ago
Assignee: general → luke
You need to log in before you can comment on or make changes to this bug.