Bug 542428 - Make wrappers aware of each other. r=jst

--HG--
extra : rebase_source : 94d8ce151abbbc5d75b1261e6a2c6cad117d047e

js/src/xpconnect/src/XPCChromeObjectWrapper.cpp

@@ -244,6 +244,9 @@ XPC_COW_Iterator(JSContext *cx, JSObject *obj, JSBool keysonly);
 static JSObject *
 XPC_COW_WrappedObject(JSContext *cx, JSObject *obj);
 
+static JSBool
+WrapFunction(JSContext *cx, JSObject *scope, JSObject *funobj, jsval *vp);
+
 using namespace XPCWrapper;
 
 namespace ChromeObjectWrapper {
@@ -275,6 +278,10 @@ JSExtendedClass COWClass = {
 JSBool
 WrapObject(JSContext *cx, JSObject *parent, jsval v, jsval *vp)
 {
+  if (JS_ObjectIsFunction(cx, JSVAL_TO_OBJECT(v))) {
+    return WrapFunction(cx, parent, JSVAL_TO_OBJECT(v), vp);
+  }
+
   JSObject *wrapperObj =
     JS_NewObjectWithGivenProto(cx, &COWClass.base, NULL, parent);
   if (!wrapperObj) {
@@ -361,9 +368,9 @@ GetWrappedObject(JSContext *cx, JSObject *wrapper)
 
 // Forward declaration for the function wrapper.
 JSBool
-XPC_COW_RewrapForChrome(JSContext *cx, JSObject *wrapperObj, jsval *vp);
+RewrapForChrome(JSContext *cx, JSObject *wrapperObj, jsval *vp);
 JSBool
-XPC_COW_RewrapForContent(JSContext *cx, JSObject *wrapperObj, jsval *vp);
+RewrapForContent(JSContext *cx, JSObject *wrapperObj, jsval *vp);
 
 // This function wrapper calls a function from untrusted content into chrome.
 
@@ -371,57 +378,51 @@ static JSBool
 XPC_COW_FunctionWrapper(JSContext *cx, JSObject *obj, uintN argc, jsval *argv,
                         jsval *rval)
 {
-  JSObject *wrappedObj;
-
-  // Allow 'this' to be either a COW, in which case we unwrap it or something
-  // that isn't a COW.  We disallow invalid COWs that have no wrapped object.
-
-  wrappedObj = GetWrapper(obj);
-  if (wrappedObj) {
-    wrappedObj = GetWrappedObject(cx, wrappedObj);
-    if (!wrappedObj) {
-      return ThrowException(NS_ERROR_ILLEGAL_VALUE, cx);
-    }
-  } else {
-    wrappedObj = obj;
-  }
-
   jsval funToCall;
   if (!JS_GetReservedSlot(cx, JSVAL_TO_OBJECT(argv[-2]),
                           XPCWrapper::eWrappedFunctionSlot, &funToCall)) {
     return JS_FALSE;
   }
 
+  JSObject *scope = JS_GetGlobalForObject(cx, JSVAL_TO_OBJECT(funToCall));
   for (uintN i = 0; i < argc; ++i) {
-    if (!XPC_COW_RewrapForChrome(cx, obj, &argv[i])) {
+    if (!JSVAL_IS_PRIMITIVE(argv[i]) &&
+        !RewrapObject(cx, scope, JSVAL_TO_OBJECT(argv[i]), UNKNOWN, &argv[i])) {
       return JS_FALSE;
     }
   }
 
-  if (!JS_CallFunctionValue(cx, wrappedObj, funToCall, argc, argv, rval)) {
+  if (!RewrapObject(cx, scope, obj, UNKNOWN, rval) ||
+      !JS_CallFunctionValue(cx, JSVAL_TO_OBJECT(*rval), funToCall, argc, argv,
+                            rval)) {
     return JS_FALSE;
   }
 
-  return XPC_COW_RewrapForContent(cx, obj, rval);
+  return RewrapForContent(cx, obj, rval);
 }
 
-JSBool
+static JSBool
-XPC_COW_WrapFunction(JSContext *cx, JSObject *outerObj, JSObject *funobj,
+WrapFunction(JSContext *cx, JSObject *scope, JSObject *funobj, jsval *rval)
-                     jsval *rval)
 {
+  scope = JS_GetGlobalForObject(cx, scope);
   jsval funobjVal = OBJECT_TO_JSVAL(funobj);
   JSFunction *wrappedFun =
     reinterpret_cast<JSFunction *>(xpc_GetJSPrivate(funobj));
   JSNative native = JS_GetFunctionNative(cx, wrappedFun);
-  if (!native || native == XPC_COW_FunctionWrapper) {
+  if (native == XPC_COW_FunctionWrapper) {
-    *rval = funobjVal;
+    if (STOBJ_GET_PARENT(funobj) == scope) {
-    return JS_TRUE;
+      *rval = funobjVal;
+      return JS_TRUE;
+    }
+
+    JS_GetReservedSlot(cx, funobj, XPCWrapper::eWrappedFunctionSlot, &funobjVal);
+    funobj = JSVAL_TO_OBJECT(funobjVal);
   }
 
   JSFunction *funWrapper =
     JS_NewFunction(cx, XPC_COW_FunctionWrapper,
                    JS_GetFunctionArity(wrappedFun), 0,
-                   JS_GetGlobalForObject(cx, outerObj),
+                   scope,
                    JS_GetFunctionName(wrappedFun));
   if (!funWrapper) {
     return JS_FALSE;
@@ -436,63 +437,32 @@ XPC_COW_WrapFunction(JSContext *cx, JSObject *outerObj, JSObject *funobj,
 }
 
 JSBool
-XPC_COW_RewrapForChrome(JSContext *cx, JSObject *wrapperObj, jsval *vp)
+RewrapForChrome(JSContext *cx, JSObject *wrapperObj, jsval *vp)
 {
   jsval v = *vp;
   if (JSVAL_IS_PRIMITIVE(v)) {
     return JS_TRUE;
   }
 
-  // We're rewrapping for chrome, so this is safe.
+  return RewrapObject(cx, JS_GetGlobalForObject(cx, GetWrappedObject(cx, wrapperObj)),
-  JSObject *obj = GetWrappedJSObject(cx, JSVAL_TO_OBJECT(v));
+                      JSVAL_TO_OBJECT(v), UNKNOWN, vp);
-  if (!obj) {
-    *vp = JSVAL_NULL;
-    return JS_TRUE;
-  }
-
-  XPCWrappedNative *wn;
-  JSBool ok;
-
-  // Set aside the frame chain so that we'll be able to wrap this object for
-  // chrome's use.
-  JSStackFrame *fp = JS_SaveFrameChain(cx);
-
-  if (IS_WN_WRAPPER(obj) &&
-      (wn = (XPCWrappedNative*)xpc_GetJSPrivate(obj)) &&
-      !nsXPCWrappedJSClass::IsWrappedJS(wn->Native())) {
-    // Return an explicit XPCNativeWrapper in case "chrome" code happens to be
-    // XBL code cloned into an untrusted context.
-    ok = XPCNativeWrapper::CreateExplicitWrapper(cx, wn, JS_TRUE, vp);
-  } else {
-    // Note: we're passing the wrapped chrome object as the scope for the SJOW.
-    ok = XPCSafeJSObjectWrapper::WrapObject(cx, GetWrappedObject(cx, wrapperObj),
-                                            *vp, vp);
-  }
-
-  JS_RestoreFrameChain(cx, fp);
-
-  return ok;
 }
 
 JSBool
-XPC_COW_RewrapForContent(JSContext *cx, JSObject *wrapperObj, jsval *vp)
+RewrapForContent(JSContext *cx, JSObject *wrapperObj, jsval *vp)
 {
   jsval v = *vp;
   if (JSVAL_IS_PRIMITIVE(v)) {
     return JS_TRUE;
   }
 
-  JSObject *obj = GetWrappedJSObject(cx, JSVAL_TO_OBJECT(v));
+  JSObject *scope = JS_GetScopeChain(cx);
-  if (!obj) {
+  if (!scope) {
-    *vp = JSVAL_NULL;
+    return JS_FALSE;
-    return JS_TRUE;
   }
 
-  if (JS_ObjectIsFunction(cx, obj)) {
+  return RewrapObject(cx, JS_GetGlobalForObject(cx, scope),
-    return XPC_COW_WrapFunction(cx, wrapperObj, obj, vp);
+                      JSVAL_TO_OBJECT(v), COW, vp);
-  }
-
-  return WrapObject(cx, JS_GetScopeChain(cx), OBJECT_TO_JSVAL(obj), vp);
 }
 
 static JSBool
@@ -549,7 +519,7 @@ XPC_COW_AddProperty(JSContext *cx, JSObject *obj, jsval id, jsval *vp)
     }
   }
 
-  return XPC_COW_RewrapForChrome(cx, obj, vp) &&
+  return RewrapForChrome(cx, obj, vp) &&
          JS_DefinePropertyById(cx, wrappedObj, interned_id, *vp,
                                desc.getter, desc.setter, desc.attrs);
 }
@@ -632,7 +602,7 @@ XPC_COW_GetOrSetProperty(JSContext *cx, JSObject *obj, jsval id, jsval *vp,
     return ThrowException(NS_ERROR_XPC_SECURITY_MANAGER_VETO, cx);
   }
 
-  if (!XPC_COW_RewrapForChrome(cx, obj, vp)) {
+  if (isSet && !RewrapForChrome(cx, obj, vp)) {
     return JS_FALSE;
   }
 
@@ -643,7 +613,7 @@ XPC_COW_GetOrSetProperty(JSContext *cx, JSObject *obj, jsval id, jsval *vp,
     return JS_FALSE;
   }
 
-  return XPC_COW_RewrapForContent(cx, obj, vp);
+  return RewrapForContent(cx, obj, vp);
 }
 
 static JSBool
@@ -714,7 +684,8 @@ XPC_COW_NewResolve(JSContext *cx, JSObject *obj, jsval idval, uintN flags,
     return ThrowException(NS_ERROR_XPC_SECURITY_MANAGER_VETO, cx);
   }
 
-  return XPCWrapper::NewResolve(cx, obj, JS_TRUE, wrappedObj, id, flags, objp);
+  return XPCWrapper::NewResolve(cx, obj, JS_FALSE, wrappedObj, id, flags,
+                                objp);
 }
 
 static JSBool
@@ -743,7 +714,7 @@ XPC_COW_Convert(JSContext *cx, JSObject *obj, JSType type, jsval *vp)
     return JS_FALSE;
   }
 
-  return XPC_COW_RewrapForContent(cx, obj, vp);
+  return RewrapForContent(cx, obj, vp);
 }
 
 static JSBool

js/src/xpconnect/src/XPCNativeWrapper.cpp

@@ -235,59 +235,33 @@ RewrapIfDeepWrapper(JSContext *cx, JSObject *obj, jsval v, jsval *rval)
   // Re-wrap non-primitive values if this is a deep wrapper, i.e.
   // if (HAS_FLAGS(flags, FLAG_DEEP).
   if (HAS_FLAGS(flags, FLAG_DEEP) && !primitive) {
-    // Unwrap a cross origin wrapper, since we're more restrictive.
+    JSObject *scope = JS_GetScopeChain(cx);
-    if (STOBJ_GET_CLASS(nativeObj) == &XPCCrossOriginWrapper::XOWClass.base) {
+    if (!scope) {
-      if (!::JS_GetReservedSlot(cx, nativeObj, sWrappedObjSlot,
-                                &v)) {
-        return JS_FALSE;
-      }
-
-      // If v is primitive, allow nativeObj to remain a cross origin wrapper,
-      // which will fail below (since it isn't a wrapped native).
-      if (!JSVAL_IS_PRIMITIVE(v)) {
-        nativeObj = JSVAL_TO_OBJECT(v);
-      }
-    }
-
-    XPCWrappedNative* wrappedNative =
-      XPCWrappedNative::GetAndMorphWrappedNativeOfJSObject(cx, nativeObj);
-    if (!wrappedNative) {
-      return XPCSafeJSObjectWrapper::WrapObject(cx, JS_GetScopeChain(cx),
-                                                v, rval);
-    }
-
-    if (HAS_FLAGS(flags, FLAG_EXPLICIT)) {
-#ifdef DEBUG_XPCNativeWrapper
-      printf("Rewrapping for deep explicit wrapper\n");
-#endif
-      if (wrappedNative == XPCNativeWrapper::SafeGetWrappedNative(obj)) {
-        // Already wrapped, return the wrapper.
-        *rval = OBJECT_TO_JSVAL(obj);
-        return JS_TRUE;
-      }
-
-      // |obj| is an explicit deep wrapper.  We want to construct another
-      // explicit deep wrapper for |v|.
-
-      return XPCNativeWrapper::CreateExplicitWrapper(cx, wrappedNative,
-                                                     JS_TRUE, rval);
-    }
-
-#ifdef DEBUG_XPCNativeWrapper
-    printf("Rewrapping for deep implicit wrapper\n");
-#endif
-    // Just using GetNewOrUsed on the return value of
-    // GetWrappedNativeOfJSObject will give the right thing -- the unique deep
-    // implicit wrapper associated with wrappedNative.
-    JSObject* wrapperObj = XPCNativeWrapper::GetNewOrUsed(cx, wrappedNative,
-                                                          JS_GetScopeChain(cx),
-                                                          nsnull);
-    if (!wrapperObj) {
       return JS_FALSE;
     }
 
-    *rval = OBJECT_TO_JSVAL(wrapperObj);
+    WrapperType type = HAS_FLAGS(flags, FLAG_EXPLICIT)
+                       ? XPCNW_EXPLICIT : XPCNW_IMPLICIT;
+
+    if (!RewrapObject(cx, JS_GetGlobalForObject(cx, scope),
+                      nativeObj, type, rval)) {
+      return JS_FALSE;
+    }
   } else {
+    if (!JSVAL_IS_PRIMITIVE(v)) {
+      JSObject *scope = JS_GetScopeChain(cx);
+      if (!scope) {
+        return JS_FALSE;
+      }
+
+      // NB: Because we're not a deep wrapper, we give a hint of SJOW to
+      // imitate not having a wrapper at all.
+      if (!RewrapObject(cx, JS_GetGlobalForObject(cx, scope),
+                        JSVAL_TO_OBJECT(v), SJOW, &v)) {
+        return JS_FALSE;
+      }
+    }
+
     *rval = v;
   }
 
@@ -460,6 +434,7 @@ EnsureLegalActivity(JSContext *cx, JSObject *obj,
   // Otherwise, we're looking at a non-system file with a handle on an
   // implicit wrapper. This is a bug! Deny access.
   NS_ERROR("Implicit native wrapper in content code");
+  return JS_FALSE;
 #else
   return JS_TRUE;
 #endif
@@ -976,49 +951,49 @@ XPCNativeWrapperCtor(JSContext *cx, JSObject *obj, uintN argc, jsval *argv,
 
   JSObject *nativeObj = JSVAL_TO_OBJECT(native);
 
-  // Unwrap a cross origin wrapper, since we're more restrictive than it is.
+  // First, if this is another type of security wrapper, unwrap it to see what
-  JSObject *wrapper;
+  // we're really dealing with.
-  if ((wrapper = UnwrapGeneric(cx, &XPCCrossOriginWrapper::XOWClass, nativeObj))) {
+  nativeObj = UnsafeUnwrapSecurityWrapper(cx, nativeObj);
-    nativeObj = wrapper;
+  if (!nativeObj) {
-  } else if ((wrapper = UnwrapGeneric(cx, &XPCSafeJSObjectWrapper::SJOWClass, nativeObj))) {
+    return ThrowException(NS_ERROR_INVALID_ARG, cx);
-    nativeObj = wrapper;
+  }
+  native = OBJECT_TO_JSVAL(nativeObj);
+
+  // Now, figure out if we're allowed to create an XPCNativeWrapper around it.
+  JSObject *scope = JS_GetScopeChain(cx);
+  if (!scope) {
+    return JS_FALSE;
   }
 
+  XPCWrappedNativeScope *xpcscope =
+    XPCWrappedNativeScope::FindInJSObjectScope(cx, scope);
+  NS_ASSERTION(xpcscope, "what crazy scope are we in?");
+
   XPCWrappedNative *wrappedNative;
+  WrapperType type = xpcscope->GetWrapperFor(cx, nativeObj, XPCNW_EXPLICIT,
+                                             &wrappedNative);
 
-  if (XPCNativeWrapper::IsNativeWrapper(nativeObj)) {
+  if (type != NONE && !(type & XPCNW_EXPLICIT)) {
-    // We're asked to wrap an already wrapped object. Re-wrap the
+    return ThrowException(NS_ERROR_INVALID_ARG, cx);
-    // object wrapped by the given wrapper.
+  }
 
-#ifdef DEBUG_XPCNativeWrapper
+  // We might have to morph.
-    printf("Wrapping already wrapped object\n");
+  if (!wrappedNative) {
-#endif
-
-    // It's always safe to re-wrap an object.
-    wrappedNative = XPCNativeWrapper::SafeGetWrappedNative(nativeObj);
-
-    if (!wrappedNative) {
-      return ThrowException(NS_ERROR_INVALID_ARG, cx);
-    }
-
-    nativeObj = wrappedNative->GetFlatJSObject();
-    native = OBJECT_TO_JSVAL(nativeObj);
-  } else {
     wrappedNative =
       XPCWrappedNative::GetAndMorphWrappedNativeOfJSObject(cx, nativeObj);
 
     if (!wrappedNative) {
       return ThrowException(NS_ERROR_INVALID_ARG, cx);
     }
+  }
 
-    // Prevent wrapping a double-wrapped JS object in an
+  // Prevent wrapping a double-wrapped JS object in an
-    // XPCNativeWrapper!
+  // XPCNativeWrapper!
-    nsCOMPtr<nsIXPConnectWrappedJS> xpcwrappedjs =
+  nsCOMPtr<nsIXPConnectWrappedJS> xpcwrappedjs =
-      do_QueryWrappedNative(wrappedNative);
+    do_QueryWrappedNative(wrappedNative);
 
-    if (xpcwrappedjs) {
+  if (xpcwrappedjs) {
-      return ThrowException(NS_ERROR_INVALID_ARG, cx);
+    return ThrowException(NS_ERROR_INVALID_ARG, cx);
-    }
   }
 
   PRBool hasStringArgs = PR_FALSE;
@@ -1057,8 +1032,16 @@ XPCNativeWrapperCtor(JSContext *cx, JSObject *obj, uintN argc, jsval *argv,
     }
   }
 
-  return XPCNativeWrapper::CreateExplicitWrapper(cx, wrappedNative,
+  if (!XPCNativeWrapper::CreateExplicitWrapper(cx, wrappedNative,
-                                                 !hasStringArgs, rval);
+                                               !hasStringArgs, rval)) {
+    return JS_FALSE;
+  }
+
+  if (!(type & SOW)) {
+    return JS_TRUE;
+  }
+
+  return SystemOnlyWrapper::MakeSOW(cx, JSVAL_TO_OBJECT(*rval));
 }
 
 static void

js/src/xpconnect/src/XPCSafeJSObjectWrapper.cpp

@@ -94,18 +94,18 @@ XPC_SJOW_Iterator(JSContext *cx, JSObject *obj, JSBool keysonly);
 static JSObject *
 XPC_SJOW_WrappedObject(JSContext *cx, JSObject *obj);
 
+using namespace XPCSafeJSObjectWrapper;
+using namespace XPCWrapper;
+
 static inline
 JSBool
 ThrowException(nsresult ex, JSContext *cx)
 {
-  XPCThrower::Throw(ex, cx);
+  DoThrowException(ex, cx);
 
   return JS_FALSE;
 }
 
-using namespace XPCSafeJSObjectWrapper;
-using namespace XPCWrapper;
-
 // Find the subject and object principal. The argument
 // subjectPrincipal can be null if the caller doesn't care about the
 // subject principal, and secMgr can also be null if the caller
@@ -274,8 +274,59 @@ JSExtendedClass SJOWClass = {
 JSBool
 WrapObject(JSContext *cx, JSObject *scope, jsval v, jsval *vp)
 {
-  *vp = v;
+  // This might be redundant if called from XPC_SJOW_Construct, but it should
-  return XPC_SJOW_Construct(cx, scope, 1, vp, vp);
+  // be cheap in that case.
+  JSObject *objToWrap = UnsafeUnwrapSecurityWrapper(cx, JSVAL_TO_OBJECT(v));
+  if (!objToWrap) {
+    return ThrowException(NS_ERROR_INVALID_ARG, cx);
+  }
+
+  // Prevent script created Script objects from ever being wrapped
+  // with XPCSafeJSObjectWrapper, and never let the eval function
+  // object be directly wrapped.
+
+  if (STOBJ_GET_CLASS(objToWrap) == &js_ScriptClass ||
+      (JS_ObjectIsFunction(cx, objToWrap) &&
+       JS_GetFunctionFastNative(cx, JS_ValueToFunction(cx, v)) ==
+       XPCWrapper::sEvalNative)) {
+    return ThrowException(NS_ERROR_INVALID_ARG, cx);
+  }
+
+  XPCWrappedNativeScope *xpcscope =
+    XPCWrappedNativeScope::FindInJSObjectScope(cx, scope);
+  NS_ASSERTION(xpcscope, "what crazy scope are we in?");
+
+  XPCWrappedNative *wrappedNative;
+  WrapperType type = xpcscope->GetWrapperFor(cx, objToWrap, SJOW,
+                                             &wrappedNative);
+
+  // NB: We allow XOW here because we're as restrictive as it is (and we know
+  // we're same origin here).
+  if (type != NONE && type != XOW && !(type & SJOW)) {
+    return ThrowException(NS_ERROR_INVALID_ARG, cx);
+  }
+
+  SLIM_LOG_WILL_MORPH(cx, objToWrap);
+  if (IS_SLIM_WRAPPER(objToWrap) && !MorphSlimWrapper(cx, objToWrap)) {
+    return ThrowException(NS_ERROR_FAILURE, cx);
+  }
+
+  JSObject *wrapperObj =
+    JS_NewObjectWithGivenProto(cx, &SJOWClass.base, nsnull, scope);
+
+  if (!wrapperObj) {
+    // JS_NewObjectWithGivenProto already threw.
+    return JS_FALSE;
+  }
+
+  *vp = OBJECT_TO_JSVAL(wrapperObj);
+  if (!JS_SetReservedSlot(cx, wrapperObj, XPCWrapper::sWrappedObjSlot,
+                          OBJECT_TO_JSVAL(objToWrap)) ||
+      !JS_SetReservedSlot(cx, wrapperObj, XPCWrapper::sFlagsSlot, JSVAL_ZERO)) {
+    return JS_FALSE;
+  }
+
+  return JS_TRUE;
 }
 
 PRBool
@@ -347,21 +398,16 @@ WrapJSValue(JSContext *cx, JSObject *obj, jsval val, jsval *rval)
   if (JSVAL_IS_PRIMITIVE(val)) {
     *rval = val;
   } else {
+    if (!RewrapObject(cx, STOBJ_GET_PARENT(obj), JSVAL_TO_OBJECT(val), SJOW,
+                      rval)) {
+      return JS_FALSE;
+    }
     // Construct a new safe wrapper. Note that it doesn't matter what
     // parent we pass in here, the construct hook will ensure we get
     // the right parent for the wrapper.
-    JSObject *safeObj =
+    JSObject *safeObj = JSVAL_TO_OBJECT(*rval);
-      ::JS_ConstructObjectWithArguments(cx, &SJOWClass.base, nsnull,
+    if (STOBJ_GET_CLASS(safeObj) == &SJOWClass.base &&
-                                        nsnull, 1, &val);
+        JS_GetGlobalForObject(cx, obj) != JS_GetGlobalForObject(cx, safeObj)) {
-    if (!safeObj) {
-      return JS_FALSE;
-    }
-
-    // Set *rval to safeObj here to ensure it doesn't get collected in
-    // any of the code below.
-    *rval = OBJECT_TO_JSVAL(safeObj);
-
-    if (JS_GetGlobalForObject(cx, obj) != JS_GetGlobalForObject(cx, safeObj)) {
       // Check to see if the new object we just wrapped is accessible
       // from the unsafe object we got the new object through. If not,
       // force the new wrapper to use the principal of the unsafe
@@ -423,21 +469,6 @@ WrapJSValue(JSContext *cx, JSObject *obj, jsval val, jsval *rval)
   return ok;
 }
 
-static jsval
-UnwrapJSValue(JSContext *cx, jsval val)
-{
-  if (JSVAL_IS_PRIMITIVE(val)) {
-    return val;
-  }
-
-  JSObject *unsafeObj = GetUnsafeObject(cx, JSVAL_TO_OBJECT(val));
-  if (unsafeObj) {
-    return OBJECT_TO_JSVAL(unsafeObj);
-  }
-
-  return val;
-}
-
 static JSBool
 XPC_SJOW_AddProperty(JSContext *cx, JSObject *obj, jsval id, jsval *vp)
 {
@@ -470,6 +501,16 @@ XPC_SJOW_AddProperty(JSContext *cx, JSObject *obj, jsval id, jsval *vp)
     return JS_FALSE;
   }
 
+  if (!JSVAL_IS_PRIMITIVE(*vp)) {
+    // Adding an object of some type to the content object, make sure it's
+    // properly wrapped.
+    JSObject *added = JSVAL_TO_OBJECT(*vp);
+    if (!RewrapObject(cx, JS_GetGlobalForObject(cx, unsafeObj), added,
+                      UNKNOWN, vp)) {
+      return JS_FALSE;
+    }
+  }
+
   return XPCWrapper::AddProperty(cx, obj, JS_FALSE, unsafeObj, id, vp);
 }
 
@@ -571,8 +612,11 @@ XPC_SJOW_GetOrSetProperty(JSContext *cx, JSObject *obj, jsval id, jsval *vp,
       return JS_FALSE;
     }
 
-    if (aIsSet) {
+    if (aIsSet &&
-      *vp = UnwrapJSValue(cx, *vp);
+        !JSVAL_IS_PRIMITIVE(*vp) &&
+        !RewrapObject(cx, JS_GetGlobalForObject(cx, unsafeObj),
+                      JSVAL_TO_OBJECT(*vp), UNKNOWN, vp)) {
+      return JS_FALSE;
     }
 
     JSBool ok = aIsSet
@@ -657,7 +701,8 @@ XPC_SJOW_NewResolve(JSContext *cx, JSObject *obj, jsval id, uintN flags,
                              XPC_SJOW_toString, 0, 0) != nsnull;
   }
 
-  return XPCWrapper::NewResolve(cx, obj, JS_FALSE, unsafeObj, id, flags, objp);
+  return XPCWrapper::NewResolve(cx, obj, JS_FALSE, unsafeObj, id, flags,
+                                objp);
 }
 
 static JSBool
@@ -778,11 +823,21 @@ XPC_SJOW_Call(JSContext *cx, JSObject *obj, uintN argc, jsval *argv,
   {
     SafeCallGuard guard(cx, FindObjectPrincipals(cx, safeObj, funToCall));
 
+    JSObject *scope = JS_GetGlobalForObject(cx, funToCall);
     for (uintN i = 0; i < argc; ++i) {
-      argv[i] = UnwrapJSValue(cx, argv[i]);
+      // NB: Passing NONE for a hint here.
+      if (!JSVAL_IS_PRIMITIVE(argv[i]) &&
+          !RewrapObject(cx, scope, JSVAL_TO_OBJECT(argv[i]), NONE, &argv[i])) {
+        return JS_FALSE;
+      }
     }
 
-    if (!JS_CallFunctionValue(cx, callThisObj, OBJECT_TO_JSVAL(funToCall),
+    jsval v;
+    if (!RewrapObject(cx, scope, callThisObj, NONE, &v)) {
+      return JS_FALSE;
+    }
+
+    if (!JS_CallFunctionValue(cx, JSVAL_TO_OBJECT(v), OBJECT_TO_JSVAL(funToCall),
                               argc, argv, rval)) {
       return JS_FALSE;
     }
@@ -814,55 +869,18 @@ XPC_SJOW_Construct(JSContext *cx, JSObject *obj, uintN argc, jsval *argv,
     return JS_TRUE;
   }
 
-  JSObject *objToWrap = JSVAL_TO_OBJECT(argv[0]);
+  JSObject *objToWrap = UnsafeUnwrapSecurityWrapper(cx, JSVAL_TO_OBJECT(argv[0]));
-
+  if (!objToWrap) {
-  // Prevent script created Script objects from ever being wrapped
-  // with XPCSafeJSObjectWrapper, and never let the eval function
-  // object be directly wrapped.
-
-  if (STOBJ_GET_CLASS(objToWrap) == &js_ScriptClass ||
-      (::JS_ObjectIsFunction(cx, objToWrap) &&
-       ::JS_GetFunctionFastNative(cx, ::JS_ValueToFunction(cx, argv[0])) ==
-       XPCWrapper::sEvalNative)) {
     return ThrowException(NS_ERROR_INVALID_ARG, cx);
   }
 
-  SLIM_LOG_WILL_MORPH(cx, objToWrap);
-  if (IS_SLIM_WRAPPER(objToWrap) && !MorphSlimWrapper(cx, objToWrap)) {
-    return ThrowException(NS_ERROR_FAILURE, cx);
-  }
-
   // Check that the caller can access the unsafe object.
   if (!CanCallerAccess(cx, nsnull, objToWrap)) {
     // CanCallerAccess() already threw for us.
     return JS_FALSE;
   }
 
-  JSObject *unsafeObj = GetUnsafeObject(cx, objToWrap);
+  return WrapObject(cx, scope, OBJECT_TO_JSVAL(objToWrap), rval);
-
-  if (unsafeObj) {
-    // We're asked to wrap an already wrapped object. Re-wrap the
-    // object wrapped by the given wrapper.
-
-    objToWrap = unsafeObj;
-  }
-
-  JSObject *wrapperObj =
-    JS_NewObjectWithGivenProto(cx, &SJOWClass.base, nsnull, scope);
-
-  if (!wrapperObj) {
-    // JS_NewObjectWithGivenProto already threw.
-    return JS_FALSE;
-  }
-
-  *rval = OBJECT_TO_JSVAL(wrapperObj);
-  if (!JS_SetReservedSlot(cx, wrapperObj, XPCWrapper::sWrappedObjSlot,
-                          OBJECT_TO_JSVAL(objToWrap)) ||
-      !JS_SetReservedSlot(cx, wrapperObj, XPCWrapper::sFlagsSlot, JSVAL_ZERO)) {
-    return JS_FALSE;
-  }
-
-  return JS_TRUE;
 }
 
 static JSBool
@@ -885,7 +903,21 @@ XPC_SJOW_Create(JSContext *cx, JSObject *obj, uintN argc, jsval *argv,
       return JS_FALSE;
     }
 
-    if (!JS_CallFunctionValue(cx, obj, OBJECT_TO_JSVAL(callee),
+    JSObject *scope = JS_GetGlobalForObject(cx, unsafeObj);
+    for (uintN i = 0; i < argc; ++i) {
+      // NB: Passing NONE for a hint here.
+      if (!JSVAL_IS_PRIMITIVE(argv[i]) &&
+          !RewrapObject(cx, scope, JSVAL_TO_OBJECT(argv[i]), NONE, &argv[i])) {
+        return JS_FALSE;
+      }
+    }
+
+    jsval v;
+    if (!RewrapObject(cx, scope, obj, NONE, &v)) {
+      return JS_FALSE;
+    }
+
+    if (!JS_CallFunctionValue(cx, JSVAL_TO_OBJECT(v), OBJECT_TO_JSVAL(unsafeObj),
                               argc, argv, rval)) {
       return JS_FALSE;
     }
@@ -944,20 +976,6 @@ XPC_SJOW_Iterator(JSContext *cx, JSObject *obj, JSBool keysonly)
     return nsnull;
   }
 
-  JSObject *tmp =
-    XPCWrapper::UnwrapGeneric(cx, &XPCCrossOriginWrapper::XOWClass,
-                              unsafeObj);
-  if (tmp) {
-    unsafeObj = tmp;
-
-    // Repeat the CanCallerAccess check because the XOW is parented to our
-    // scope's global object which makes the above CanCallerAccess call lie.
-    if (!CanCallerAccess(cx, nsnull, unsafeObj)) {
-      // CanCallerAccess() already threw for us.
-      return nsnull;
-    }
-  }
-
   // Create our dummy SJOW.
   JSObject *wrapperIter =
     JS_NewObjectWithGivenProto(cx, &SJOWClass.base, nsnull,

js/src/xpconnect/src/XPCSystemOnlyWrapper.cpp

@@ -555,7 +555,7 @@ XPC_SOW_NewResolve(JSContext *cx, JSObject *obj, jsval id, uintN flags,
     return JS_FALSE;
   }
 
-  return NewResolve(cx, obj, JS_TRUE, wrappedObj, id, flags, objp);
+  return NewResolve(cx, obj, JS_FALSE, wrappedObj, id, flags, objp);
 }
 
 static JSBool

js/src/xpconnect/src/XPCWrapper.cpp

@@ -181,19 +181,11 @@ JSBool
 RewrapObject(JSContext *cx, JSObject *scope, JSObject *obj, WrapperType hint,
              jsval *vp)
 {
-  if (IsSecurityWrapper(obj)) {
+  obj = UnsafeUnwrapSecurityWrapper(cx, obj);
-    jsval v;
+  if (!obj) {
-    JS_GetReservedSlot(cx, obj, sWrappedObjSlot, &v);
+    // A wrapper wrapping NULL (such as XPCNativeWrapper.prototype).
-    NS_ASSERTION(!JSVAL_IS_PRIMITIVE(v), "bad object");
+    *vp = JSVAL_NULL;
-    obj = JSVAL_TO_OBJECT(v);
+    return JS_TRUE;
-  } else if (XPCNativeWrapper::IsNativeWrapper(obj)) {
-    XPCWrappedNative *wn = XPCNativeWrapper::SafeGetWrappedNative(obj);
-    if (!wn) {
-      *vp = JSVAL_NULL;
-      return JS_TRUE;
-    }
-
-    obj = wn->GetFlatJSObject();
   }
 
   XPCWrappedNativeScope *nativescope =
@@ -210,6 +202,28 @@ RewrapObject(JSContext *cx, JSObject *scope, JSObject *obj, WrapperType hint,
   return CreateWrapperFromType(cx, scope, wn, answer, vp);
 }
 
+JSObject *
+UnsafeUnwrapSecurityWrapper(JSContext *cx, JSObject *obj)
+{
+  if (IsSecurityWrapper(obj)) {
+    jsval v;
+    JS_GetReservedSlot(cx, obj, sWrappedObjSlot, &v);
+    NS_ASSERTION(!JSVAL_IS_PRIMITIVE(v), "bad object");
+    return JSVAL_TO_OBJECT(v);
+  }
+
+  if (XPCNativeWrapper::IsNativeWrapper(obj)) {
+    XPCWrappedNative *wn = XPCNativeWrapper::SafeGetWrappedNative(obj);
+    if (!wn) {
+      return nsnull;
+    }
+
+    return wn->GetFlatJSObject();
+  }
+
+  return obj;
+}
+
 JSBool
 CreateWrapperFromType(JSContext *cx, JSObject *scope, XPCWrappedNative *wn,
                       WrapperType hint, jsval *vp)

js/src/xpconnect/src/XPCWrapper.h

@@ -424,6 +424,9 @@ JSBool
 RewrapObject(JSContext *cx, JSObject *scope, JSObject *obj, WrapperType hint,
              jsval *vp);
 
+JSObject *
+UnsafeUnwrapSecurityWrapper(JSContext *cx, JSObject *obj);
+
 JSBool
 CreateWrapperFromType(JSContext *cx, JSObject *scope, XPCWrappedNative *wn,
                       WrapperType hint, jsval *vp);
@@ -463,11 +466,14 @@ Enumerate(JSContext *cx, JSObject *wrapperObj, JSObject *innerObj);
  * Resolves a property (that may be) defined on |innerObj| onto
  * |wrapperObj|. This will also resolve random, page-defined objects
  * and is therefore unsuitable for cross-origin resolution.
+ *
+ * If |caller| is not NONE, then we will call the proper WrapObject
+ * hook for any getters or setters about to be lifted onto
+ * |wrapperObj|.
  */
 JSBool
-NewResolve(JSContext *cx, JSObject *wrapperObj,
+NewResolve(JSContext *cx, JSObject *wrapperObj, JSBool preserveVal,
-           JSBool preserveVal, JSObject *innerObj,
+           JSObject *innerObj, jsval id, uintN flags, JSObject **objp);
-           jsval id, uintN flags, JSObject **objp);
 
 /**
  * Resolve a native property named id from innerObj onto wrapperObj. The

js/src/xpconnect/src/xpcwrappednativescope.cpp

@@ -1053,15 +1053,19 @@ XPCWrappedNativeScope::GetWrapperFor(JSContext *cx, JSObject *obj,
             XPCCrossOriginWrapper::ClassNeedsXOW(obj->getClass()->name);
 
         // Is other a chrome object?
+        JSObject *obj2;
+        XPCWrappedNative *wrapper =
+            XPCWrappedNative::GetWrappedNativeOfJSObject(cx, obj, nsnull, &obj2);
         if(principalEqual || obj->isSystem())
-            return (hint & XPCNW) ? hint : wantsXOW ? SJOW : NONE;
+        {
+            if(hint & XPCNW)
+                return (wrapper || obj2) ? hint : NONE;
+            return wantsXOW ? SJOW : NONE;
+        }
 
         // Other isn't a chrome object: we need to wrap it in a SJOW or an
         // XPCNW.
 
-        JSObject *obj2;
-        XPCWrappedNative *wrapper =
-            XPCWrappedNative::GetWrappedNativeOfJSObject(cx, obj, nsnull, &obj2);
         if(!wrapper && !obj2)
             hint = SJOW;
 
@@ -1105,9 +1109,13 @@ XPCWrappedNativeScope::GetWrapperFor(JSContext *cx, JSObject *obj,
     // any other types of wrapper than the hint.
     if(!wrapper && !obj2)
     {
-        NS_ASSERTION(principalEqual,
+#if 0
+        // XXX Re-enable these assertions when we have a better mochitest
+        // solution than UniversalXPConnect.
+        NS_ASSERTION(principalEqual || hint == COW,
                      "touching non-wrappednative object cross origin?");
-        NS_ASSERTION(hint == SJOW || hint == UNKNOWN, "bad hint");
+        NS_ASSERTION(hint == SJOW || hint == COW || hint == UNKNOWN, "bad hint");
+#endif
         return hint;
     }
 
@@ -1132,7 +1140,10 @@ XPCWrappedNativeScope::GetWrapperFor(JSContext *cx, JSObject *obj,
     if(!principalEqual ||
        XPCCrossOriginWrapper::ClassNeedsXOW(obj->getClass()->name))
     {
-        NS_ASSERTION(hint != SJOW, "shouldn't have a SJOW for cross origin access?");
+        // NB: We want to assert that hint is not SJOW here, but it can
+        // be because of shallow XPCNativeWrappers. In that case, XOW is
+        // the right return value because XPCNativeWrappers are meant for
+        // chrome, and we're in content which shouldn't expect SJOWs.
         return (hint & XPCNW) ? XPCNW_EXPLICIT : XOW;
     }
 

js/src/xpconnect/tests/chrome/Makefile.in

@@ -48,6 +48,7 @@ _CHROME_FILES = \
 		bug503926.xul \
 		test_bug503926.xul \
 		test_bug533596.xul \
+		test_wrappers.xul \
 		$(NULL)
 
 libs:: $(_CHROME_FILES)

js/src/xpconnect/tests/chrome/bug503926.xul

@@ -18,9 +18,9 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=503926
       var passed = false;
       var obj = { QueryInterface: function() { passed = true; } }
       try { document.documentElement.appendChild(obj); } catch (e) {}
-      var isDialog = location.hash != 'iframe';
+      var isDialog = location.hash != '#iframe';
-      var outer = isDialog ? opener.wrappedJSObject : top.wrappedJSObject;
+      var outer = XPCNativeWrapper.unwrap(isDialog ? opener : top);
-      outer.ok(passed, "chrome/chrome test passed");
+      outer.ok(passed, "chrome/chrome test passed: " + (isDialog ? "dialog" : "iframe"));
       if (isDialog)
         close();
   ]]>

js/src/xpconnect/tests/chrome/test_bug503926.xul

@@ -15,10 +15,10 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=503926
   <body xmlns="http://www.w3.org/1999/xhtml">
   <a href="https://bugzilla.mozilla.org/show_bug.cgi?id=503926"
      target="_blank">Mozilla Bug 503926</a>
-  </body>
 
   <iframe id="ifr" type="content" onload="iframe_loaded()"
-          href="chrome://mochikit/content/chrome/js/src/xpconnect/tests/chrome/bug503926.xul#iframe"/>
+          src="chrome://mochikit/content/chrome/js/src/xpconnect/tests/chrome/bug503926.xul#iframe"/>
+  </body>
 
   <!-- test code goes here -->
   <script type="application/javascript">
@@ -32,7 +32,7 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=503926
     ok(passed, "trusted QIs should be called");
 
     openDialog("chrome://mochikit/content/chrome/js/src/xpconnect/tests/chrome/bug503926.xul",
-               "chromeDialog", "modal")
+               "chromeDialog", "modal");
     SimpleTest.finish();
   }
 

js/src/xpconnect/tests/chrome/test_bug533596.xul

@@ -32,7 +32,8 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=533596
     is(o, XPCNativeWrapper.unwrap(o), "unwrap on a random object returns it");
 
     var win = $('ifr').contentWindow;
-    var utils = window.getInterface(Components.interfaces.nsIDOMWindowUtils);
+    var utils = window.QueryInterface(Components.interfaces.nsIInterfaceRequestor)
+                      .getInterface(Components.interfaces.nsIDOMWindowUtils);
     is(utils.getClassName(win), "XPCNativeWrapper", "win is an XPCNW");
     ok("x" in XPCNativeWrapper.unwrap(win), "actually unwrapped");
     is(utils.getClassName(XPCNativeWrapper.unwrap(win)), "XPCSafeJSObjectWrapper",

js/src/xpconnect/tests/chrome/test_wrappers.xul

@@ -0,0 +1,58 @@
+<?xml version="1.0"?>
+<?xml-stylesheet href="chrome://global/skin" type="text/css"?>
+<?xml-stylesheet href="chrome://mochikit/content/tests/SimpleTest/test.css"
+                 type="text/css"?>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=500931
+-->
+<window title="Mozilla Bug 500931"
+  xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
+  <script type="application/javascript"
+          src="chrome://mochikit/content/MochiKit/packed.js"></script>
+  <script type="application/javascript"
+          src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
+
+  <!-- test results are displayed in the html:body -->
+  <body xmlns="http://www.w3.org/1999/xhtml">
+  <a href="https://bugzilla.mozilla.org/show_bug.cgi?id=533596"
+     target="_blank">Mozilla Bug 533596</a>
+  </body>
+
+  <!-- test code goes here -->
+  <script type="application/javascript"><![CDATA[
+
+  /** Test for Bug 533596 **/
+
+  function go() {
+    var win = $('ifr').contentWindow;
+    var utils = window.QueryInterface(Components.interfaces.nsIInterfaceRequestor)
+                      .getInterface(Components.interfaces.nsIDOMWindowUtils);
+    todo_is(utils.getClassName(window), "XPCSafeJSObjectWrapper", "our window is wrapped correctly")
+    todo_is(utils.getClassName(location), "XPCSafeJSObjectWrapper", "our location is wrapped correctly")
+    is(utils.getClassName(win), "XPCNativeWrapper", "win is an XPCNW");
+    is(utils.getClassName(win.location), "XPCNativeWrapper", "deep wrapping works");
+    is(win.location.href, "http://example.org/tests/js/src/xpconnect/tests/mochitest/chrome_wrappers_helper.html",
+       "can still get strings out");
+
+    var unsafeWin = win.wrappedJSObject;
+    is(utils.getClassName(unsafeWin), "XPCSafeJSObjectWrapper", "can get a SJOW");
+    is(utils.getClassName(unsafeWin.location), "XPCSafeJSObjectWrapper", "deep wrapping works");
+
+    unsafeWin.run_test(ok, win, unsafeWin);
+
+    win.setTimeout(function() {
+                       is(utils.getClassName(this), "XPCNativeWrapper",
+                          "this is wrapped correctly");
+                       SimpleTest.finish();
+                   }, 0)
+  }
+
+  SimpleTest.waitForExplicitFinish();
+
+  ]]></script>
+  <iframe type="content"
+          src="http://example.org/tests/js/src/xpconnect/tests/mochitest/chrome_wrappers_helper.html"
+          onload="go()"
+          id="ifr">
+  </iframe>
+</window>

js/src/xpconnect/tests/mochitest/Makefile.in

@@ -48,6 +48,7 @@ _TEST_FILES =	bug500931_helper.html \
 		inner.html \
 		bug92773_helper.html \
 		bug504877_helper.html \
+		chrome_wrappers_helper.html \
 		test_bug92773.html \
 		test_bug361111.xul \
 		test_bug384632.html \

js/src/xpconnect/tests/mochitest/chrome_wrappers_helper.html

@@ -0,0 +1,21 @@
+<html>
+    <head>
+        <script>
+            function check_wrapper(ok, wrapper, expected, note) {
+                netscape.security.PrivilegeManager.enablePrivilege("UniversalXPConnect");
+                var utils = window.QueryInterface(Components.interfaces.nsIInterfaceRequestor)
+                                  .getInterface(Components.interfaces.nsIDOMWindowUtils);
+                ok(utils.getClassName(wrapper) === expected, note);
+            }
+            function run_test(ok, xpcnw, sjow) {
+                // both wrappers should point to our window: XOW
+                check_wrapper(ok, ok, "Function", "functions are wrapped properly")
+                ok(ok.__parent__ == window, "ok is parented correctly");
+                check_wrapper(ok, xpcnw, "XPCCrossOriginWrapper", "XPCNWs are transformed correctly");
+                check_wrapper(ok, sjow, "XPCCrossOriginWrapper", "SJOWs are transformed correctly");
+            }
+        </script>
+    </head>
+    <body>
+    </body>
+</html>

js/src/xpconnect/tests/mochitest/test_cows.html

@@ -148,6 +148,13 @@ function COWTests() {
         todo(false, "COWed functions should not raise " + e);
     }
 
+    try {
+        var obj = {
+            get prop() { return { __exposedProps__: {}, test: "FAIL" } }
+        };
+        ok(getCOW(obj).prop.test != "FAIL", "getting prop.test should throw");
+    } catch (e) {}
+
     try {
         var objWithFunc = {__exposedProps__: {foo: 'r'},
                            foo: function foo() { return 5; }};