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

RESOLVED DUPLICATE of bug 560796

Status

()

--
critical
RESOLVED DUPLICATE of bug 560796
9 years ago
6 years ago

People

(Reporter: jruderman, 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

9 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

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

Comment 2

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

Comment 3

9 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)
(Assignee)

Comment 4

9 years ago
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

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

Comment 6

9 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 :)
(Assignee)

Comment 7

9 years ago
(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

9 years ago
Assignee: general → jwalden+bmo
Whiteboard: [sg:critical?] → [sg:critical?][critsmash:investigating]
Duplicate of this bug: 561309
Jeff, any progress on this?
Jeff?
(Assignee)

Comment 12

9 years ago
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: 9 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.