Bug 1101123. Don't allow redefining the getter of a non-configurable accessor property on native objects, even via the low-level JSAPI methods. r=efaust,bholley

This commit is contained in:
Boris Zbarsky 2014-12-09 14:44:38 -05:00
parent 19e3190828
commit 212b09964c
4 changed files with 79 additions and 6 deletions

View File

@ -974,6 +974,13 @@ class MOZ_STACK_CLASS SourceBufferHolder MOZ_FINAL
#define JSFUN_CONSTRUCTOR 0x400 /* native that can be called as a ctor */
#define JSPROP_REDEFINE_NONCONFIGURABLE 0x800 /* If set, will allow redefining a
non-configurable property, but
only on a non-DOM global. This
is a temporary hack that will
need to go away in bug
1105518 */
#define JSPROP_IGNORE_ENUMERATE 0x1000 /* ignore the value in JSPROP_ENUMERATE.
This flag only valid when defining over
an existing property. */

View File

@ -1393,6 +1393,37 @@ PurgeScopeChain(ExclusiveContext *cx, HandleObject obj, HandleId id)
return true;
}
/*
* Check whether we're redefining away a non-configurable getter, and
* throw if so.
*/
static inline bool
CheckAccessorRedefinition(ExclusiveContext *cx, HandleObject obj, HandleShape shape,
PropertyOp getter, StrictPropertyOp setter, HandleId id, unsigned attrs)
{
MOZ_ASSERT(shape->isAccessorDescriptor());
if (shape->configurable() || (getter == shape->getter() && setter == shape->setter()))
return true;
/*
* Only allow redefining if JSPROP_REDEFINE_NONCONFIGURABLE is set _and_
* the object is a non-DOM global. The idea is that a DOM object can
* never have such a thing on its proto chain directly on the web, so we
* should be OK optimizing access to accessors found on such an object.
*/
if ((attrs & JSPROP_REDEFINE_NONCONFIGURABLE) &&
obj->is<GlobalObject>() &&
!obj->getClass()->isDOMClass())
{
return true;
}
if (!cx->isJSContext())
return false;
return Throw(cx->asJSContext(), id, JSMSG_CANT_REDEFINE_PROP);
}
bool
js::DefineNativeProperty(ExclusiveContext *cx, HandleNativeObject obj, HandleId id, HandleValue value,
PropertyOp getter, StrictPropertyOp setter, unsigned attrs)
@ -1428,6 +1459,8 @@ js::DefineNativeProperty(ExclusiveContext *cx, HandleNativeObject obj, HandleId
shape = obj->lookup(cx, id);
}
if (shape->isAccessorDescriptor()) {
if (!CheckAccessorRedefinition(cx, obj, shape, getter, setter, id, attrs))
return false;
attrs = ApplyOrDefaultAttributes(attrs, shape);
shape = NativeObject::changeProperty<SequentialExecution>(cx, obj, shape, attrs,
JSPROP_GETTER | JSPROP_SETTER,
@ -1454,8 +1487,15 @@ js::DefineNativeProperty(ExclusiveContext *cx, HandleNativeObject obj, HandleId
* loops.
*/
shape = obj->lookup(cx, id);
if (shape && shape->isDataDescriptor())
attrs = ApplyOrDefaultAttributes(attrs, shape);
if (shape) {
if (shape->isAccessorDescriptor() &&
!CheckAccessorRedefinition(cx, obj, shape, getter, setter, id, attrs))
{
return false;
}
if (shape->isDataDescriptor())
attrs = ApplyOrDefaultAttributes(attrs, shape);
}
} else {
/*
* We have been asked merely to update some attributes by a caller of
@ -1481,6 +1521,12 @@ js::DefineNativeProperty(ExclusiveContext *cx, HandleNativeObject obj, HandleId
shape = obj->lookup(cx, id);
}
if (shape->isAccessorDescriptor() &&
!CheckAccessorRedefinition(cx, obj, shape, getter, setter, id, attrs))
{
return false;
}
attrs = ApplyOrDefaultAttributes(attrs, shape);
/* Keep everything from the shape that isn't the things we're changing */

View File

@ -446,7 +446,7 @@ sandbox_addProperty(JSContext *cx, HandleObject obj, HandleId id, MutableHandleV
return false;
unsigned attrs = pd.attributes() & ~(JSPROP_GETTER | JSPROP_SETTER);
if (!JS_DefinePropertyById(cx, obj, id, vp,
attrs | JSPROP_PROPOP_ACCESSORS,
attrs | JSPROP_PROPOP_ACCESSORS | JSPROP_REDEFINE_NONCONFIGURABLE,
JS_PROPERTYOP_GETTER(writeToProto_getProperty),
JS_PROPERTYOP_SETTER(writeToProto_setProperty)))
return false;

View File

@ -335,7 +335,8 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=933681
[Symbol.for("registrySymbolProp")]: 44`;
}
var trickyObject =
iwin.eval(`new Object({
iwin.eval(`(function() {
var o = new Object({
primitiveProp: 42, objectProp: { foo: 2 },
xoProp: top.location, hasOwnProperty: 10,
get getterProp() { return 2; },
@ -345,7 +346,11 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=933681
callableProp: function() { },
nonXrayableProp: new WeakMap()
${symbolProps}
})`);
});
Object.defineProperty(o, "nonConfigurableGetterSetterProp",
{ get: function() { return 5; }, set: function() {} });
return o;
})()`);
testTrickyObject(trickyObject);
}
@ -373,7 +378,8 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=933681
trickyArray.hasOwnProperty = 10;
Object.defineProperty(trickyArray, 'getterProp', { get: function() { return 2; }});
Object.defineProperty(trickyArray, 'setterProp', { set: function(x) {}});
Object.defineProperty(trickyArray, 'getterSetterProp', { get: function() { return 3; }, set: function(x) {}});
Object.defineProperty(trickyArray, 'getterSetterProp', { get: function() { return 3; }, set: function(x) {}, configurable: true});
Object.defineProperty(trickyArray, 'nonConfigurableGetterSetterProp', { get: function() { return 5; }, set: function(x) {}});
trickyArray.callableProp = function() {};
trickyArray.nonXrayableProp = new WeakMap();
${symbolProps}
@ -487,6 +493,20 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=933681
is(trickyObject.getterSetterProp, 'redefined', "Redefinition works");
is(trickyObject.wrappedJSObject.getterSetterProp, 'redefined', "Redefinition forwards");
// We should NOT be able to overwrite an existing non-configurable accessor
// prop, though.
is(trickyObject.wrappedJSObject.nonConfigurableGetterSetterProp, 5,
"Underlying object has getter");
is(trickyObject.nonConfigurableGetterSetterProp, undefined,
"Filtering properly over Xray here too");
checkThrows(function() {
trickyObject.nonConfigurableGetterSetterProp = 'redefined';
}, /config/, "Should throw when redefining non-configurable prop");
is(trickyObject.nonConfigurableGetterSetterProp, undefined,
"Redefinition should have thrown");
is(trickyObject.wrappedJSObject.nonConfigurableGetterSetterProp, 5,
"Redefinition really should have thrown");
checkThrows(function() { trickyObject.hasOwnProperty = 33; }, /shadow/,
"Should reject shadowing of pre-existing inherited properties over Xrays");