Bug 957688 - Remove side-effect-free calls to js::CheckAccess. r=mrbkap

js::CheckAccess has all sorts of crazy side-effects on its parameters. Luckily,
they mostly happen on dead values.

We have to alter a jit-test that previously threw, and doesn't anymore. I have
confirmed that the reason for throwing was not the security check itself, but
rather the lookupGeneric call that happens inside js::CheckAccess, which ends
up throwing 'undefined is not a function'. It seems like this is just an issue
of calling lookupGeneric when we shouldn't, and that the correct behavior here
is not to throw.
This commit is contained in:
Bobby Holley 2014-01-24 16:08:24 -08:00
parent 10c01ae603
commit 12b60887b5
5 changed files with 2 additions and 54 deletions

View File

@ -572,11 +572,6 @@ obj_watch(JSContext *cx, unsigned argc, Value *vp)
if (!ValueToId<CanGC>(cx, args[0], &propid))
return false;
RootedValue tmp(cx);
unsigned attrs;
if (!CheckAccess(cx, obj, propid, JSACC_WATCH, &tmp, &attrs))
return false;
if (!JSObject::watch(cx, obj, propid, callable))
return false;

View File

@ -1,3 +1,2 @@
// |jit-test| error: TypeError
z = Proxy.create({}, (function(){}));
({__proto__: z, set c(a) {}});

View File

@ -583,15 +583,6 @@ DefinePropertyOnObject(JSContext *cx, HandleObject obj, HandleId id, const PropD
JS_ASSERT(desc.isAccessorDescriptor());
/*
* Getters and setters are just like watchpoints from an access
* control point of view.
*/
RootedValue dummy(cx);
unsigned dummyAttrs;
if (!CheckAccess(cx, obj, id, JSACC_WATCH, &dummy, &dummyAttrs))
return false;
RootedValue tmp(cx, UndefinedValue());
return baseops::DefineGeneric(cx, obj, id, tmp,
desc.getter(), desc.setter(), desc.attributes());
@ -815,14 +806,6 @@ DefinePropertyOnObject(JSContext *cx, HandleObject obj, HandleId id, const PropD
} else {
JS_ASSERT(desc.isAccessorDescriptor());
/*
* Getters and setters are just like watchpoints from an access
* control point of view.
*/
RootedValue dummy(cx);
if (!CheckAccess(cx, obj2, id, JSACC_WATCH, &dummy, &attrs))
return false;
/* 8.12.9 step 12. */
unsigned changed = 0;
if (desc.hasConfigurable())
@ -5253,15 +5236,6 @@ bool
js::WatchGuts(JSContext *cx, JS::HandleObject origObj, JS::HandleId id, JS::HandleObject callable)
{
RootedObject obj(cx, GetInnerObject(cx, origObj));
if (origObj != obj) {
// If by unwrapping and innerizing, we changed the object, check again
// to make sure that we're allowed to set a watch point.
RootedValue v(cx);
unsigned attrs;
if (!CheckAccess(cx, obj, id, JSACC_WATCH, &v, &attrs))
return false;
}
if (obj->isNative()) {
// Use sparse indexes for watched objects, as dense elements can be
// written to without checking the watchpoint map.

View File

@ -3917,19 +3917,9 @@ js::InitGetterSetterOperation(JSContext *cx, jsbytecode *pc, HandleObject obj, H
HandleObject val)
{
JS_ASSERT(val->isCallable());
/*
* Getters and setters are just like watchpoints from an access control
* point of view.
*/
RootedValue scratch(cx, UndefinedValue());
unsigned attrs = 0;
if (!CheckAccess(cx, obj, id, JSACC_WATCH, &scratch, &attrs))
return false;
PropertyOp getter;
StrictPropertyOp setter;
attrs = JSPROP_ENUMERATE | JSPROP_SHARED;
unsigned attrs = JSPROP_ENUMERATE | JSPROP_SHARED;
JSOp op = JSOp(*pc);
@ -3944,7 +3934,7 @@ js::InitGetterSetterOperation(JSContext *cx, jsbytecode *pc, HandleObject obj, H
attrs |= JSPROP_SETTER;
}
scratch.setUndefined();
RootedValue scratch(cx);
return JSObject::defineGeneric(cx, obj, id, scratch, getter, setter, attrs);
}

View File

@ -305,9 +305,6 @@ JS_SetWatchPoint(JSContext *cx, JSObject *obj_, jsid id_,
if (!obj)
return false;
RootedValue v(cx);
unsigned attrs;
RootedId propid(cx);
if (JSID_IS_INT(id)) {
@ -321,13 +318,6 @@ JS_SetWatchPoint(JSContext *cx, JSObject *obj_, jsid id_,
return false;
}
/*
* If, by unwrapping and innerizing, we changed the object, check
* again to make sure that we're allowed to set a watch point.
*/
if (origobj != obj && !CheckAccess(cx, obj, propid, JSACC_WATCH, &v, &attrs))
return false;
if (!obj->isNative()) {
JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_CANT_WATCH,
obj->getClass()->name);