Fixing bug 422205. Make XOW/SJOW wrappers do security checks on enumeration and interation. r+sr=mrbkap@gmail.com
authorjst@mozilla.org
Tue, 18 Mar 2008 14:17:58 -0700
changeset 13270 a7bf38a290491d8a5c2bb29c97740b7cd15fa90d
parent 13269 b1d9492b9c395ce137e0b69af9336b2725f5cccf
child 13271 47ce12b031ed09a7696f92fb5dfecb552015102c
push idunknown
push userunknown
push dateunknown
bugs422205
milestone1.9b5pre
Fixing bug 422205. Make XOW/SJOW wrappers do security checks on enumeration and interation. r+sr=mrbkap@gmail.com
js/src/xpconnect/src/XPCCrossOriginWrapper.cpp
js/src/xpconnect/src/XPCSafeJSObjectWrapper.cpp
js/src/xpconnect/src/XPCWrapper.cpp
--- a/js/src/xpconnect/src/XPCCrossOriginWrapper.cpp
+++ b/js/src/xpconnect/src/XPCCrossOriginWrapper.cpp
@@ -1039,41 +1039,52 @@ XPC_XOW_Equality(JSContext *cx, JSObject
   test = other->GetFlatJSObject();
   return ((JSExtendedClass *)STOBJ_GET_CLASS(obj))->
     equality(cx, obj, OBJECT_TO_JSVAL(test), bp);
 }
 
 JS_STATIC_DLL_CALLBACK(JSObject *)
 XPC_XOW_Iterator(JSContext *cx, JSObject *obj, JSBool keysonly)
 {
+  JSObject *wrappedObj = GetWrappedObject(cx, obj);
+  if (!wrappedObj) {
+    ThrowException(NS_ERROR_INVALID_ARG, cx);
+    return nsnull;
+  }
+  nsresult rv = IsWrapperSameOrigin(cx, wrappedObj);
+  if (NS_FAILED(rv)) {
+    if (rv == NS_ERROR_DOM_PROP_ACCESS_DENIED) {
+      // Can't create iterators for foreign objects.
+      ThrowException(rv, cx);
+      return nsnull;
+    }
+
+    ThrowException(NS_ERROR_FAILURE, cx);
+    return nsnull;
+  }
+
   JSObject *wrapperIter = JS_NewObject(cx, &sXPC_XOW_JSClass.base, nsnull,
                                        JS_GetGlobalForObject(cx, obj));
   if (!wrapperIter) {
     return nsnull;
   }
 
   JSAutoTempValueRooter tvr(cx, OBJECT_TO_JSVAL(wrapperIter));
 
   // Initialize our XOW.
-  JSObject *innerObj = GetWrappedObject(cx, obj);
-  if (!innerObj) {
-    ThrowException(NS_ERROR_INVALID_ARG, cx);
-    return nsnull;
-  }
-
-  jsval v = OBJECT_TO_JSVAL(innerObj);
+  jsval v = OBJECT_TO_JSVAL(wrappedObj);
   if (!JS_SetReservedSlot(cx, wrapperIter, XPCWrapper::sWrappedObjSlot, v) ||
       !JS_SetReservedSlot(cx, wrapperIter, XPCWrapper::sResolvingSlot,
                           JSVAL_FALSE) ||
       !JS_SetReservedSlot(cx, wrapperIter, XPC_XOW_ScopeSlot,
                           PRIVATE_TO_JSVAL(nsnull))) {
     return nsnull;
   }
 
-  return XPCWrapper::CreateIteratorObj(cx, wrapperIter, obj, innerObj,
+  return XPCWrapper::CreateIteratorObj(cx, wrapperIter, obj, wrappedObj,
                                        keysonly);
 }
 
 JS_STATIC_DLL_CALLBACK(JSObject *)
 XPC_XOW_WrappedObject(JSContext *cx, JSObject *obj)
 {
   return GetWrappedObject(cx, obj);
 }
--- a/js/src/xpconnect/src/XPCSafeJSObjectWrapper.cpp
+++ b/js/src/xpconnect/src/XPCSafeJSObjectWrapper.cpp
@@ -613,16 +613,22 @@ XPC_SJOW_Enumerate(JSContext *cx, JSObje
   // enumerated identifiers from the unsafe object to the safe
   // wrapper.
 
   JSObject *unsafeObj = GetUnsafeObject(obj);
   if (!unsafeObj) {
     return JS_TRUE;
   }
 
+  // Check that the caller can access the unsafe object.
+  if (!CanCallerAccess(cx, unsafeObj)) {
+    // CanCallerAccess() already threw for us.
+    return JS_FALSE;
+  }
+
   // Since we enumerate using JS_Enumerate() on the unsafe object here
   // we don't need to do a security check since JS_Enumerate() will
   // look up unsafeObj.__iterator__ and if we don't have permission to
   // access that, it'll throw and we'll be safe.
 
   return XPCWrapper::Enumerate(cx, obj, unsafeObj);
 }
 
@@ -968,38 +974,49 @@ XPC_SJOW_Equality(JSContext *cx, JSObjec
   }
 
   return JS_TRUE;
 }
 
 JS_STATIC_DLL_CALLBACK(JSObject *)
 XPC_SJOW_Iterator(JSContext *cx, JSObject *obj, JSBool keysonly)
 {
-  JSObject *innerObj = GetUnsafeObject(obj);
-  if (!innerObj) {
+  obj = FindSafeObject(obj);
+  NS_ASSERTION(obj != nsnull, "FindSafeObject() returned null in class hook!");
+
+  JSObject *unsafeObj = GetUnsafeObject(obj);
+  if (!unsafeObj) {
     ThrowException(NS_ERROR_INVALID_ARG, cx);
+
+    return nsnull;
+  }
+
+  // Check that the caller can access the unsafe object.
+  if (!CanCallerAccess(cx, unsafeObj)) {
+    // CanCallerAccess() already threw for us.
     return nsnull;
   }
 
   // Create our dummy SJOW.
   JSObject *wrapperIter =
-    ::JS_NewObjectWithGivenProto(cx, &sXPC_SJOW_JSClass.base, nsnull, innerObj);
+    ::JS_NewObjectWithGivenProto(cx, &sXPC_SJOW_JSClass.base, nsnull,
+                                 unsafeObj);
   if (!wrapperIter) {
     return nsnull;
   }
 
   if (!::JS_SetReservedSlot(cx, wrapperIter, XPC_SJOW_SLOT_IS_RESOLVING,
                             BOOLEAN_TO_JSVAL(JS_FALSE))) {
     return nsnull;
   }
 
   JSAutoTempValueRooter tvr(cx, OBJECT_TO_JSVAL(wrapperIter));
 
   // Initialize the wrapper.
-  return XPCWrapper::CreateIteratorObj(cx, wrapperIter, obj, innerObj,
+  return XPCWrapper::CreateIteratorObj(cx, wrapperIter, obj, unsafeObj,
                                        keysonly);
 }
 
 JS_STATIC_DLL_CALLBACK(JSObject *)
 XPC_SJOW_WrappedObject(JSContext *cx, JSObject *obj)
 {
   return GetUnsafeObject(obj);
 }
--- a/js/src/xpconnect/src/XPCWrapper.cpp
+++ b/js/src/xpconnect/src/XPCWrapper.cpp
@@ -335,17 +335,17 @@ XPCWrapper::NewResolve(JSContext *cx, JS
   uintN attrs = JSPROP_ENUMERATE;
   JSPropertyOp getter = nsnull;
   JSPropertyOp setter = nsnull;
   if (isXOW && OBJ_IS_NATIVE(innerObjp)) {
     JSScopeProperty *sprop = reinterpret_cast<JSScopeProperty *>(prop);
 
     attrs = sprop->attrs;
     if (attrs & JSPROP_GETTER) {
-      getter =  sprop->getter;
+      getter = sprop->getter;
     }
     if (attrs & JSPROP_SETTER) {
       setter = sprop->setter;
     }
 
     if (preserveVal && SPROP_HAS_VALID_SLOT(sprop, OBJ_SCOPE(innerObjp))) {
       v = OBJ_GET_SLOT(cx, innerObjp, sprop->slot);
     }