Bug 583850 - Assert that certain security checks in the JS engine never fail. r=mrbkap.
authorJason Orendorff <jorendorff@mozilla.com>
Mon, 02 Aug 2010 16:38:46 -0500
changeset 48807 c5e31473b1a670f26b865a49390613d74b3c30a3
parent 48806 c6131ed87e9ce7433153c43c4bb7275c35cb4453
child 48808 617b92ed8ced5c32228b5bf6b420d44afec5aef3
child 50518 d796055d54653e1f30b08d833958bdcd79f544bd
push id14825
push userrsayre@mozilla.com
push dateWed, 04 Aug 2010 07:47:43 +0000
treeherdermozilla-central@c761f8e85b8c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmrbkap
bugs583850
milestone2.0b3pre
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 583850 - Assert that certain security checks in the JS engine never fail. r=mrbkap.
js/src/jsinterp.cpp
js/src/jsobj.cpp
--- a/js/src/jsinterp.cpp
+++ b/js/src/jsinterp.cpp
@@ -5861,24 +5861,23 @@ BEGIN_CASE(JSOP_SETTER)
         JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL,
                              JSMSG_BAD_GETTER_OR_SETTER,
                              (op == JSOP_GETTER)
                              ? js_getter_str
                              : js_setter_str);
         goto error;
     }
 
-    /*
-     * Getters and setters are just like watchpoints from an access control
-     * point of view.
-     */
+    /* Legacy security check. This can't fail. See bug 583850. */
     Value rtmp;
     uintN attrs;
-    if (!CheckAccess(cx, obj, id, JSACC_WATCH, &rtmp, &attrs))
+    if (!CheckAccess(cx, obj, id, JSACC_WATCH, &rtmp, &attrs)) {
+        JS_NOT_REACHED("getter/setter access check failed");
         goto error;
+    }
 
     PropertyOp getter, setter;
     if (op == JSOP_GETTER) {
         getter = CastAsPropertyOp(&rval.toObject());
         setter = PropertyStub;
         attrs = JSPROP_GETTER;
     } else {
         getter = PropertyStub;
--- a/js/src/jsobj.cpp
+++ b/js/src/jsobj.cpp
@@ -137,17 +137,21 @@ static JSPropertySpec object_props[] = {
 };
 
 static JSBool
 obj_getProto(JSContext *cx, JSObject *obj, jsid id, Value *vp)
 {
     /* Let CheckAccess get the slot's value, based on the access mode. */
     uintN attrs;
     id = ATOM_TO_JSID(cx->runtime->atomState.protoAtom);
-    return CheckAccess(cx, obj, id, JSACC_PROTO, vp, &attrs);
+
+    /* Legacy security check. This can't fail. See bug 583850. */
+    JSBool ok = CheckAccess(cx, obj, id, JSACC_PROTO, vp, &attrs);
+    JS_ASSERT(ok);
+    return ok;
 }
 
 static JSBool
 obj_setProto(JSContext *cx, JSObject *obj, jsid id, Value *vp)
 {
     if (!vp->isObjectOrNull())
         return JS_TRUE;
 
@@ -158,20 +162,23 @@ obj_setProto(JSContext *cx, JSObject *ob
          * outer object. This ensures that any with statements only grant
          * access to the inner object.
          */
         OBJ_TO_INNER_OBJECT(cx, pobj);
         if (!pobj)
             return JS_FALSE;
     }
 
+    /* Legacy security check. This can't fail. See bug 583850. */
     uintN attrs;
     id = ATOM_TO_JSID(cx->runtime->atomState.protoAtom);
-    if (!CheckAccess(cx, obj, id, JSAccessMode(JSACC_PROTO|JSACC_WRITE), vp, &attrs))
+    if (!CheckAccess(cx, obj, id, JSAccessMode(JSACC_PROTO|JSACC_WRITE), vp, &attrs)) {
+        JS_NOT_REACHED("setProto access check failed");
         return JS_FALSE;
+    }
 
     return SetProto(cx, obj, pobj, JS_TRUE);
 }
 
 #else  /* !JS_HAS_OBJ_PROTO_PROP */
 
 #define object_props NULL
 
@@ -1293,40 +1300,46 @@ obj_watch_handler(JSContext *cx, JSObjec
     return ok;
 }
 
 static JSBool
 obj_watch(JSContext *cx, uintN argc, Value *vp)
 {
     if (argc <= 1) {
         js_ReportMissingArg(cx, *vp, 1);
-        return JS_FALSE;
+        return false;
     }
 
     JSObject *callable = js_ValueToCallableObject(cx, &vp[3], 0);
     if (!callable)
-        return JS_FALSE;
+        return false;
 
     /* Compute the unique int/atom symbol id needed by js_LookupProperty. */
     jsid propid;
     if (!ValueToId(cx, vp[2], &propid))
-        return JS_FALSE;
+        return false;
 
     JSObject *obj = ComputeThisFromVp(cx, vp);
+    if (!obj)
+        return false;
+
+    /* Legacy security check. This can't fail. See bug 583850. */
     Value tmp;
     uintN attrs;
-    if (!obj || !CheckAccess(cx, obj, propid, JSACC_WATCH, &tmp, &attrs))
-        return JS_FALSE;
+    if (!CheckAccess(cx, obj, propid, JSACC_WATCH, &tmp, &attrs)) {
+        JS_NOT_REACHED("watchpoint access check failed");
+        return false;
+    }
 
     vp->setUndefined();
 
     if (attrs & JSPROP_READONLY)
-        return JS_TRUE;
+        return true;
     if (obj->isDenseArray() && !obj->makeDenseArraySlow(cx))
-        return JS_FALSE;
+        return false;
     return JS_SetWatchPoint(cx, obj, propid, obj_watch_handler, callable);
 }
 
 static JSBool
 obj_unwatch(JSContext *cx, uintN argc, Value *vp)
 {
     JSObject *obj = ComputeThisFromVp(cx, vp);
     if (!obj)
@@ -1525,24 +1538,24 @@ js_obj_defineGetter(JSContext *cx, uintN
     PropertyOp getter = CastAsPropertyOp(&vp[3].toObject());
 
     jsid id;
     if (!ValueToId(cx, vp[2], &id))
         return JS_FALSE;
     JSObject *obj = ComputeThisFromVp(cx, vp);
     if (!obj || !CheckRedeclaration(cx, obj, id, JSPROP_GETTER, NULL, NULL))
         return JS_FALSE;
-    /*
-     * Getters and setters are just like watchpoints from an access
-     * control point of view.
-     */
+
+    /* Legacy security check. This can't fail. See bug 583850. */
     Value junk;
     uintN attrs;
-    if (!CheckAccess(cx, obj, id, JSACC_WATCH, &junk, &attrs))
+    if (!CheckAccess(cx, obj, id, JSACC_WATCH, &junk, &attrs)) {
+        JS_NOT_REACHED("defineGetter access check failed");
         return JS_FALSE;
+    }
     vp->setUndefined();
     return obj->defineProperty(cx, id, UndefinedValue(), getter, PropertyStub,
                                JSPROP_ENUMERATE | JSPROP_GETTER | JSPROP_SHARED);
 }
 
 JS_FRIEND_API(JSBool)
 js_obj_defineSetter(JSContext *cx, uintN argc, Value *vp)
 {
@@ -1555,24 +1568,24 @@ js_obj_defineSetter(JSContext *cx, uintN
     PropertyOp setter = CastAsPropertyOp(&vp[3].toObject());
 
     jsid id;
     if (!ValueToId(cx, vp[2], &id))
         return JS_FALSE;
     JSObject *obj = ComputeThisFromVp(cx, vp);
     if (!obj || !CheckRedeclaration(cx, obj, id, JSPROP_SETTER, NULL, NULL))
         return JS_FALSE;
-    /*
-     * Getters and setters are just like watchpoints from an access
-     * control point of view.
-     */
+
+    /* Legacy security check. This can't fail. See bug 583850. */
     Value junk;
     uintN attrs;
-    if (!CheckAccess(cx, obj, id, JSACC_WATCH, &junk, &attrs))
+    if (!CheckAccess(cx, obj, id, JSACC_WATCH, &junk, &attrs)) {
+        JS_NOT_REACHED("defineSetter access check failed");
         return JS_FALSE;
+    }
     vp->setUndefined();
     return obj->defineProperty(cx, id, UndefinedValue(), PropertyStub, setter,
                                JSPROP_ENUMERATE | JSPROP_SETTER | JSPROP_SHARED);
 }
 
 static JSBool
 obj_lookupGetter(JSContext *cx, uintN argc, Value *vp)
 {
@@ -1634,19 +1647,23 @@ obj_getPrototypeOf(JSContext *cx, uintN 
             return JS_FALSE;
         JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL,
                              JSMSG_UNEXPECTED_TYPE, bytes, "not an object");
         JS_free(cx, bytes);
         return JS_FALSE;
     }
 
     JSObject *obj = &vp[2].toObject();
+
+    /* Legacy security check. This can't fail. See bug 583850. */
     uintN attrs;
-    return CheckAccess(cx, obj, ATOM_TO_JSID(cx->runtime->atomState.protoAtom),
-                       JSACC_PROTO, vp, &attrs);
+    JSBool ok = CheckAccess(cx, obj, ATOM_TO_JSID(cx->runtime->atomState.protoAtom),
+                            JSACC_PROTO, vp, &attrs);
+    JS_ASSERT(ok);
+    return ok;
 }
 
 extern JSBool
 js_NewPropertyDescriptorObject(JSContext *cx, jsid id, uintN attrs,
                                const Value &getter, const Value &setter,
                                const Value &value, Value *vp)
 {
     /* We have our own property, so start creating the descriptor. */
@@ -1984,24 +2001,23 @@ DefinePropertyOnObject(JSContext *cx, JS
         if (desc.isGenericDescriptor() || desc.isDataDescriptor()) {
             JS_ASSERT(!obj->getOps()->defineProperty);
             return js_DefineProperty(cx, obj, desc.id, &desc.value,
                                      PropertyStub, PropertyStub, desc.attrs);
         }
 
         JS_ASSERT(desc.isAccessorDescriptor());
 
-        /*
-         * Getters and setters are just like watchpoints from an access
-         * control point of view.
-         */
+        /* Legacy security check. This can't fail. See bug 583850. */
         Value dummy;
         uintN dummyAttrs;
-        if (!CheckAccess(cx, obj, desc.id, JSACC_WATCH, &dummy, &dummyAttrs))
+        if (!CheckAccess(cx, obj, desc.id, JSACC_WATCH, &dummy, &dummyAttrs)) {
+            JS_NOT_REACHED("defineProperty access check failed");
             return JS_FALSE;
+        }
 
         Value tmp = UndefinedValue();
         return js_DefineProperty(cx, obj, desc.id, &tmp,
                                  desc.getter(), desc.setter(), desc.attrs);
     }
 
     /* 8.12.9 steps 5-6 (note 5 is merely a special case of 6). */
     Value v = UndefinedValue();
@@ -2178,24 +2194,22 @@ DefinePropertyOnObject(JSContext *cx, JS
 
         if (desc.hasValue)
             v = desc.value;
         attrs = (desc.attrs & ~unchanged) | (sprop->attributes() & unchanged);
         getter = setter = PropertyStub;
     } else {
         JS_ASSERT(desc.isAccessorDescriptor());
 
-        /*
-         * Getters and setters are just like watchpoints from an access
-         * control point of view.
-         */
+        /* Legacy security check. This can't fail. See bug 583850. */
         Value dummy;
         if (!CheckAccess(cx, obj2, desc.id, JSACC_WATCH, &dummy, &attrs)) {
-             obj2->dropProperty(cx, current);
-             return JS_FALSE;
+            JS_NOT_REACHED("defineProperty access check failed");
+            obj2->dropProperty(cx, current);
+            return JS_FALSE;
         }
 
         JS_ASSERT_IF(sprop->isMethod(), !(attrs & (JSPROP_GETTER | JSPROP_SETTER)));
 
         /* 8.12.9 step 12. */
         uintN changed = 0;
         if (desc.hasConfigurable)
             changed |= JSPROP_PERMANENT;