Crash [@ JSObject::isNative] with defineProperty on window.__proto__ (XOW?), iter

RESOLVED DUPLICATE of bug 560796

Status

()

Core
JavaScript Engine
--
critical
RESOLVED DUPLICATE of bug 560796
8 years ago
5 years ago

People

(Reporter: Jesse Ruderman, Assigned: Waldo)

Tracking

(Blocks: 1 bug, {crash, testcase})

Trunk
x86
Mac OS X
crash, testcase
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:dos], crash signature)

Attachments

(2 attachments)

(Reporter)

Description

8 years ago
Created attachment 441683 [details]
testcase (crashes Firefox when loaded)

cf bug 561309, where the property goes on the window object rather than its __proto__, and the crash happens during GC rather than during iteration.
(Reporter)

Comment 1

8 years ago
Created attachment 441684 [details]
stack trace for the crash

Comment 2

8 years ago
Ouch. From casual inspection this looks like a rooting bug in xpconnect. I will try to reproduce. ccing mrbkap.

Comment 3

8 years ago
Weird. Setter is a raw function pointer, but we think it should be an obj. waldo?

(gdb) up
#4  0x0000000104140295 in JSScopeProperty::JSScopeProperty (this=0x7fff5fbfa6d0, id=4808854276, getter=0x11e46b140, setter=0x1000c26e3 <XPC_XOW_SetProperty(JSContext*, JSObject*, long, long*)>, slot=4294967295, attrs=113, flags=0, shortid=0) at jsscope.h:678
678	        JS_ASSERT_IF(setter && (attrs & JSPROP_SETTER),
(gdb) up
#5  0x000000010415f0e8 in JSScope::addPropertyHelper (this=0x11b50cdc0, cx=0x124f76a00, id=4808854276, getter=0x11e46b140, setter=0x1000c26e3 <XPC_XOW_SetProperty(JSContext*, JSObject*, long, long*)>, slot=4294967295, attrs=113, flags=0, shortid=0, spp=0x129583908) at ../../../js/src/jsscope.cpp:825
warning: Source file is more recent than executable.
825	        JSScopeProperty child(id, getter, setter, slot, attrs, flags, shortid);
(gdb) up
#6  0x000000010415f357 in JSScope::putProperty (this=0x11b50cdc0, cx=0x124f76a00, id=4808854276, getter=0x11e46b140, setter=0x1000c26e3 <XPC_XOW_SetProperty(JSContext*, JSObject*, long, long*)>, slot=4294967295, attrs=113, flags=0, shortid=0) at ../../../js/src/jsscope.cpp:883
883	        return addPropertyHelper(cx, id, getter, setter, slot, attrs, flags, shortid, spp);
(gdb) down
#5  0x000000010415f0e8 in JSScope::addPropertyHelper (this=0x11b50cdc0, cx=0x124f76a00, id=4808854276, getter=0x11e46b140, setter=0x1000c26e3 <XPC_XOW_SetProperty(JSContext*, JSObject*, long, long*)>, slot=4294967295, attrs=113, flags=0, shortid=0, spp=0x129583908) at ../../../js/src/jsscope.cpp:825
825	        JSScopeProperty child(id, getter, setter, slot, attrs, flags, shortid);
(gdb) down
#4  0x0000000104140295 in JSScopeProperty::JSScopeProperty (this=0x7fff5fbfa6d0, id=4808854276, getter=0x11e46b140, setter=0x1000c26e3 <XPC_XOW_SetProperty(JSContext*, JSObject*, long, long*)>, slot=4294967295, attrs=113, flags=0, shortid=0) at jsscope.h:678
678	        JS_ASSERT_IF(setter && (attrs & JSPROP_SETTER),
(gdb) list
673	        : id(id), rawGetter(getter), rawSetter(setter), slot(slot), attrs(uint8(attrs)),
674	          flags(uint8(flags)), shortid(int16(shortid))
675	    {
676	        JS_ASSERT_IF(getter && (attrs & JSPROP_GETTER),
677	                     JSVAL_TO_OBJECT(getterValue())->isCallable());
678	        JS_ASSERT_IF(setter && (attrs & JSPROP_SETTER),
679	                     JSVAL_TO_OBJECT(setterValue())->isCallable());
680	    }
681	
682	    bool marked() const { return (flags & MARK) != 0; }
(gdb) p getter
$4 = (JSPropertyOp) 0x11e46b140
(gdb) p setter
$5 = (JSPropertyOp) 0x1000c26e3 <XPC_XOW_SetProperty(JSContext*, JSObject*, long, long*)>
(gdb) p (JSObject *)getter
$6 = (JSObject *) 0x11e46b140
(gdb) p *(JSObject *)getter
$7 = {
  map = 0x11b30a1a0, 
  classword = 4364931872, 
  fslots = {4795869264, 4802938752, 4795839696, 22, 22}, 
  dslots = 0x0, 
  static JSSLOT_PRIMITIVE_THIS = 2, 
  static JSSLOT_ARRAY_LENGTH = 2, 
  static JSSLOT_ARRAY_COUNT = 3, 
  static JSSLOT_ARRAY_UNUSED = 4, 
  static JSSLOT_ARGS_LENGTH = 3, 
  static JSSLOT_ARGS_CALLEE = 4, 
  static ARGS_FIXED_RESERVED_SLOTS = 2, 
  static JSSLOT_DATE_UTC_TIME = 2, 
  static JSSLOT_DATE_LOCAL_TIME = 3, 
  static DATE_FIXED_RESERVED_SLOTS = 2, 
  static JSSLOT_REGEXP_LAST_INDEX = 3, 
  static REGEXP_FIXED_RESERVED_SLOTS = 1
}
(gdb)
The problem is that in GetPropertyAttributesById, we assign desc->setter = sprop->setter().  rawSetter is NULL, and then we (reasonably?) call JS_DefineProperty with desc->setter and desc->attrs.  desc->attrs sez we have a setter function, desc->setter is NULL but that gets converted to clasp->getProperty, and assertions complain loudly.

Not sure what exactly is the right fix here...
(Reporter)

Comment 5

8 years ago
You get assertions?  I just get a crash.

Comment 6

8 years ago
Yeah this is not a mere assertion. We start using the function pointer like a jsval, something thats unlikely to result in happy times. Waldo, this is really your part of town. Want to grab it? I will help if needed :)
(gdb) p this->rawSetter
$2 = (JSPropertyOp) 0x7ffff5c5d69b <XPC_XOW_SetProperty(JSContext*, JSObject*, jsval, jsval*)>

Your XPC_XOW_SetProperty method is probably "aligned" as far as jsval tagging is concerned.

I have no problem grabbing this, I'm just not sure what the fix should be.  Making JSScopeProperty::{g,s}etter() censor to JS_PropertyStub maybe?
Don't write a raw JSPropertyOp but leave JSPROP_[SG]ETTER set in attrs!

/be
(Reporter)

Updated

8 years ago
Assignee: general → jwalden+bmo

Updated

8 years ago
Whiteboard: [sg:critical?] → [sg:critical?][critsmash:investigating]

Updated

8 years ago
Duplicate of this bug: 561309
Jeff, any progress on this?
Jeff?
Oops, I'd had some inkling that this might be fixed by bug 560796 but forgot to check back after that bug's patch got pushed -- and indeed it is.  Compare comment 4 to the change enacted there: NULL getter or setter with JSPROP_GETTER or JSPROP_SETTER in attrs passes through directly without being converted to the class {g,s}etProperty hook after that change, which directly addresses the problem noted in that comment.
Group: core-security
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → DUPLICATE
Whiteboard: [sg:critical?][critsmash:investigating] → [sg:dos]
Duplicate of bug: 560796
Crash Signature: [@ JSObject::isNative]
A testcase for this bug was already added in the original bug (bug 560796).
Flags: in-testsuite-
You need to log in before you can comment on or make changes to this bug.