Closed
Bug 566549
Opened 14 years ago
Closed 14 years ago
Crash [@ js_AllocSlot] or "Assertion failure: obj->map->ops->defineProperty == js_DefineProperty" with evalcx
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
blocking2.0 | --- | betaN+ |
blocking1.9.2 | --- | - |
status1.9.2 | --- | wontfix |
status1.9.1 | --- | unaffected |
People
(Reporter: jruderman, Assigned: jorendorff)
References
Details
(4 keywords, Whiteboard: [sg:nse][ccbr][critsmash:investigating][fixed-in-tracemonkey])
Attachments
(1 file, 1 obsolete file)
3.26 KB,
patch
|
brendan
:
review+
|
Details | Diff | Splinter Review |
evalcx('var p;', []) Assertion failure: obj->map->ops->defineProperty == js_DefineProperty, at ../jsops.cpp:2752
Comment 1•14 years ago
|
||
autoBisect shows this is probably related to the following changeset: The first bad revision is: changeset: 32011:369b7fbc2304 user: Jason Orendorff date: Wed Aug 26 14:28:36 2009 -0700 summary: Bug 508685 - Remove last parameter of defineProperty op. r=brendan.
Comment 2•14 years ago
|
||
evalcx('') Function("evalcx(\"var p\",[])")() crashes opt shell at js_AllocSlot on TM tip without -j and gives a similar assert for a debug shell. Looks dangerous (mov instruction from a weird memory address), so setting s-s and assuming [sg:critical?] unless otherwise. === (gdb) bt #0 0x0000000000465803 in js_AllocSlot () #1 0x00000000004c032f in JSScope::getChildProperty(JSContext*, JSScopeProperty*, JSScopeProperty&) () #2 0x00000000004c0607 in JSScope::addPropertyHelper(JSContext*, long, int (*)(JSContext*, JSObject*, long, long*), int (*)(JSContext*, JSObject*, long, long*), unsigned int, unsigned int, unsigned int, int, JSScopeProperty**) () #3 0x00000000004c0ace in JSScope::putProperty(JSContext*, long, int (*)(JSContext*, JSObject*, long, long*), int (*)(JSContext*, JSObject*, long, long*), unsigned int, unsigned int, unsigned int, int) () #4 0x0000000000469474 in js_DefineNativeProperty () #5 0x000000000054abcb in js_Interpret () #6 0x0000000000457b51 in js_Execute () #7 0x000000000040b815 in JS_EvaluateUCScriptForPrincipals () #8 0x000000000040b8e2 in JS_EvaluateUCScript () #9 0x00000000004071ae in EvalInContext(JSContext*, JSObject*, unsigned int, long*, long*) () #10 0x0000000000458352 in js_Invoke () #11 0x000000000054680a in js_Interpret () #12 0x0000000000457b51 in js_Execute () #13 0x000000000040b176 in JS_ExecuteScript () #14 0x00000000004062c0 in Process(JSContext*, JSObject*, char*, int) () #15 0x0000000000406ecf in main () (gdb) x/i $rip => 0x465803 <js_AllocSlot+35>: mov 0x8(%rsi),%rax (gdb) x/b $rsi 0x4179bfcc00000000: Cannot access memory at address 0x4179bfcc00000000
Group: core-security
blocking1.9.2: --- → ?
blocking2.0: --- → ?
Whiteboard: [sg:critical?][ccbr]
Comment 3•14 years ago
|
||
Comment #2 was tested on 64-bit Ubuntu Linux 10.04.
Assignee | ||
Updated•14 years ago
|
Assignee: general → jorendorff
Assignee | ||
Comment 4•14 years ago
|
||
This affects code that does *not* use JSOPTION_VAROBJFIX and that can pass a non-native object to the "obj" parameter of JS_ExecuteScript or similar APIs. So the fix is either "don't do that" (document the API restriction and fix the shell) or change JSOP_DEFVAR to cope with a non-native varobj.
Assignee | ||
Comment 5•14 years ago
|
||
This patch restores the functionality. It adds a (predictable) branch in the interpreter. No changes needed to jstracer because we don't trace these instructions. (I don't think they can occur in a loop.)
Attachment #446017 -
Flags: review?(brendan)
Comment 6•14 years ago
|
||
(In reply to comment #4) > So the fix is either "don't do that" (document the API restriction and fix the > shell) or change JSOP_DEFVAR to cope with a non-native varobj. Not looking for more non-native variable objects at this point, so let's say "don't do that". But make it stronger than doc -- assert that the obj passed into the API is native. /be
Assignee | ||
Comment 7•14 years ago
|
||
Comment on attachment 446017 [details] [diff] [review] v1 Withdrawing v1.
Attachment #446017 -
Flags: review?(brendan)
Updated•14 years ago
|
Whiteboard: [sg:critical?][ccbr] → [sg:critical?][ccbr][critsmash:investigating]
Assignee | ||
Comment 8•14 years ago
|
||
Historical note - bug 508685 comment 0 mentions this possibility, but I never followed up. :(
Assignee | ||
Comment 9•14 years ago
|
||
Attachment #446017 -
Attachment is obsolete: true
Attachment #446045 -
Flags: review?(brendan)
Comment 10•14 years ago
|
||
Comment on attachment 446045 [details] [diff] [review] v2 Could just assert initialVarObj->isNative() but I'll defer to you at this point. /be
Attachment #446045 -
Flags: review?(brendan) → review+
Assignee | ||
Comment 11•14 years ago
|
||
isNative() would be true for XML objects and With objects, at least for the time being--pending bug 561785 and bug 564936, respectively.
Assignee | ||
Comment 12•14 years ago
|
||
Try-servering, on the off chance that we expose some kind of eval-like API internally that could be tickled by this. It is, I guess, possible that this could cause a surprise or two when we switch security wrappers from the current implementation (as native objects) to proxies.
Updated•14 years ago
|
Summary: "Assertion failure: obj->map->ops->defineProperty == js_DefineProperty" with evalcx → Crash [@ js_AllocSlot] or "Assertion failure: obj->map->ops->defineProperty == js_DefineProperty" with evalcx
Updated•14 years ago
|
Assignee | ||
Comment 13•14 years ago
|
||
http://hg.mozilla.org/tracemonkey/rev/5d30188ef5a9
Whiteboard: [sg:critical?][ccbr][critsmash:investigating] → [sg:critical?][ccbr][critsmash:investigating][fixed-in-tracemonkey]
Comment 14•14 years ago
|
||
Gary found another way of hitting this: x = Proxy.create({}, this) evalcx("x", x) proxies are non-native, like the dense array above
Comment 16•14 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/5d30188ef5a9
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Updated•14 years ago
|
blocking1.9.2: .5+ → .6+
Assignee | ||
Comment 17•14 years ago
|
||
Hi! Why is this bug blocking 1.9.2.6? The patch doesn't apply cleanly to mozilla-1.9.2 and I'd rather not backport it without understanding why we want it there. All the test cases are shell-only and involve evalcx.
Comment 18•14 years ago
|
||
Generally any fixed sg-crit that has landed on trunk will get a blocking flag. Because this was found internally and of comment 17 we'll not block on this for 1.9.2.6.
Updated•14 years ago
|
blocking2.0: ? → betaN+
Comment 19•14 years ago
|
||
If this is shell-only we don't need it on the older branches, and it probably wasn't ever "sg:critical" if it was a bug in the non-shipping js-shell only.
Group: core-security
blocking1.9.2: needed → -
Whiteboard: [sg:critical?][ccbr][critsmash:investigating][fixed-in-tracemonkey] → [sg:nse][ccbr][critsmash:investigating][fixed-in-tracemonkey]
Comment 20•11 years ago
|
||
A testcase for this bug was automatically identified at js/src/tests/js1_8_5/regress/regress-566549.js.
Flags: in-testsuite+
You need to log in
before you can comment on or make changes to this bug.
Description
•