Backed out changeset 5d3af3ff9639. It doesn't allow UniversalXPConnect scripts to arbitrarily unwrap XPCNativeWrappers.
authorBlake Kaplan <mrbkap@gmail.com>
Fri, 16 Jan 2009 19:36:38 -0800
changeset 23841 cd3ec07611fc6ee4046fdaf7db92d7802f2486b1
parent 23840 5d3af3ff96391326fea5f07b9eac94ea9216cedd
child 23842 90cded872e97643771a761086a03e861cd4044e0
push id4755
push usermrbkap@mozilla.com
push dateSat, 17 Jan 2009 03:37:00 +0000
treeherdermozilla-central@cd3ec07611fc [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
milestone1.9.2a1pre
backs out5d3af3ff96391326fea5f07b9eac94ea9216cedd
Backed out changeset 5d3af3ff9639. It doesn't allow UniversalXPConnect scripts to arbitrarily unwrap XPCNativeWrappers.
dom/src/base/nsJSEnvironment.cpp
js/src/xpconnect/src/XPCNativeWrapper.cpp
js/src/xpconnect/src/XPCNativeWrapper.h
js/src/xpconnect/src/xpcquickstubs.cpp
js/src/xpconnect/src/xpcwrappednative.cpp
js/src/xpconnect/src/xpcwrappednativejsops.cpp
js/src/xpconnect/tests/mochitest/test_wrappers.html
--- a/dom/src/base/nsJSEnvironment.cpp
+++ b/dom/src/base/nsJSEnvironment.cpp
@@ -2266,16 +2266,17 @@ nsJSContext::GetGlobalObject()
     NS_WARNING("Context has no global.");
     return nsnull;
   }
 
   JSClass *c = JS_GET_CLASS(mContext, global);
 
   if (!c || ((~c->flags) & (JSCLASS_HAS_PRIVATE |
                             JSCLASS_PRIVATE_IS_NSISUPPORTS))) {
+    NS_WARNING("Global is not an nsISupports.");
     return nsnull;
   }
 
   nsCOMPtr<nsIScriptGlobalObject> sgo;
   nsISupports *priv =
     (nsISupports *)::JS_GetPrivate(mContext, global);
 
   nsCOMPtr<nsIXPConnectWrappedNative> wrapped_native =
--- a/js/src/xpconnect/src/XPCNativeWrapper.cpp
+++ b/js/src/xpconnect/src/XPCNativeWrapper.cpp
@@ -150,19 +150,17 @@ ShouldBypassNativeWrapper(JSContext *cx,
   // If there's no script, bypass for now because that's what the old code did.
   // XXX FIXME: bug 341477 covers figuring out what we _should_ do.
   return !script || !(::JS_GetScriptFilenameFlags(script) & JSFILENAME_SYSTEM);
 }
 
 #define XPC_NW_BYPASS_BASE(cx, obj, code)                                     \
   JS_BEGIN_MACRO                                                              \
     if (ShouldBypassNativeWrapper(cx, obj)) {                                 \
-      /* Use SafeGetWrappedNative since obj can't be an explicit native       \
-         wrapper. */                                                          \
-      XPCWrappedNative *wn_ = XPCNativeWrapper::SafeGetWrappedNative(obj);    \
+      XPCWrappedNative *wn_ = XPCNativeWrapper::GetWrappedNative(obj);        \
       if (!wn_) {                                                             \
         return JS_TRUE;                                                       \
       }                                                                       \
       obj = wn_->GetFlatJSObject();                                           \
       code                                                                    \
     }                                                                         \
   JS_END_MACRO
 
@@ -190,108 +188,53 @@ ThrowException(nsresult ex, JSContext *c
 
   return JS_FALSE;
 }
 
 static inline
 JSBool
 EnsureLegalActivity(JSContext *cx, JSObject *obj)
 {
-  nsIScriptSecurityManager *ssm = XPCWrapper::GetSecurityManager();
-  if (!ssm) {
-    // If there's no security manager, then we're not running in a browser
-    // context: allow access.
-    return JS_TRUE;
-  }
-
-  JSStackFrame *fp;
-  nsIPrincipal *subjectPrincipal = ssm->GetCxSubjectPrincipalAndFrame(cx, &fp);
-  if (!subjectPrincipal || !fp) {
-    // We must allow the access if there is no code running.
-    return JS_TRUE;
-  }
-
-  // This might be chrome code or content code with UniversalXPConnect.
-  void *annotation = JS_GetFrameAnnotation(cx, fp);
-  PRBool isPrivileged = PR_FALSE;
-  nsresult rv = subjectPrincipal->IsCapabilityEnabled("UniversalXPConnect",
-                                                      annotation,
-                                                      &isPrivileged);
-  if (NS_SUCCEEDED(rv) && isPrivileged) {
-    return JS_TRUE;
-  }
-
-  // We're in unprivileged code, ensure that we're allowed to access the
-  // underlying object.
-  XPCWrappedNative *wn = XPCNativeWrapper::SafeGetWrappedNative(obj);
-  if (wn) {
-    nsIPrincipal *objectPrincipal = wn->GetScope()->GetPrincipal();
-    PRBool subsumes;
-    if (NS_FAILED(subjectPrincipal->Subsumes(objectPrincipal, &subsumes)) ||
-        !subsumes) {
-      return ThrowException(NS_ERROR_XPC_SECURITY_MANAGER_VETO, cx);
-    }
-  }
-
-  // The underlying object is accessible, but this might be the wrong
-  // type of wrapper to access it through.
-  // TODO This should just be an assertion now.
   jsval flags;
 
   ::JS_GetReservedSlot(cx, obj, 0, &flags);
   if (HAS_FLAGS(flags, FLAG_EXPLICIT)) {
     // Can't make any assertions about the owner of this wrapper.
     return JS_TRUE;
   }
 
-  JSScript *script = JS_GetFrameScript(cx, fp);
-  uint32 fileFlags = JS_GetScriptFilenameFlags(script);
-  if (fileFlags == JSFILENAME_NULL || (fileFlags & JSFILENAME_SYSTEM)) {
+  JSStackFrame *frame = nsnull;
+  uint32 fileFlags = JS_GetTopScriptFilenameFlags(cx, NULL);
+  if (!JS_FrameIterator(cx, &frame) ||
+      fileFlags == JSFILENAME_NULL ||
+      (fileFlags & JSFILENAME_SYSTEM)) {
     // We expect implicit native wrappers in system files.
     return JS_TRUE;
   }
 
+  nsIScriptSecurityManager *ssm = XPCWrapper::GetSecurityManager();
+  if (!ssm) {
+    // If there's no security manager, then we're not running in a browser
+    // context: allow access.
+    return JS_TRUE;
+  }
+
+  // A last ditch effort to allow access: if the currently-running code
+  // has UniversalXPConnect privileges, then allow access.
+  PRBool isPrivileged;
+  nsresult rv = ssm->IsCapabilityEnabled("UniversalXPConnect", &isPrivileged);
+  if (NS_SUCCEEDED(rv) && isPrivileged) {
+    return JS_TRUE;
+  }
+
   // Otherwise, we're looking at a non-system file with a handle on an
   // implicit wrapper. This is a bug! Deny access.
   return ThrowException(NS_ERROR_XPC_SECURITY_MANAGER_VETO, cx);
 }
 
-// static
-JSBool
-XPCNativeWrapper::GetWrappedNative(JSContext *cx, JSObject *obj,
-                                   XPCWrappedNative **aWrappedNative)
-{
-  XPCWrappedNative *wn = static_cast<XPCWrappedNative *>(xpc_GetJSPrivate(obj));
-  *aWrappedNative = wn;
-  if (!wn) {
-    return JS_TRUE;
-  }
-
-  nsIScriptSecurityManager *ssm = XPCWrapper::GetSecurityManager();
-  if (!ssm) {
-    return JS_TRUE;
-  }
-
-  nsIPrincipal *subjectPrincipal = ssm->GetCxSubjectPrincipal(cx);
-  if (!subjectPrincipal) {
-    return JS_TRUE;
-  }
-
-  XPCWrappedNativeScope *scope = wn->GetScope();
-  nsIPrincipal *objectPrincipal = scope->GetPrincipal();
-
-  PRBool subsumes;
-  nsresult rv = subjectPrincipal->Subsumes(objectPrincipal, &subsumes);
-  if (NS_FAILED(rv) || !subsumes) {
-    return JS_FALSE;
-  }
-
-  return JS_TRUE;
-}
-
 JSBool
 XPC_NW_WrapFunction(JSContext* cx, JSObject* funobj, jsval *rval)
 {
   // If funobj is already a wrapped function, just return it.
   if (JS_GetFunctionNative(cx,
                            JS_ValueToFunction(cx, OBJECT_TO_JSVAL(funobj))) ==
       XPC_NW_FunctionWrapper) {
     *rval = OBJECT_TO_JSVAL(funobj);
@@ -425,17 +368,17 @@ XPC_NW_RewrapIfDeepWrapper(JSContext *cx
       *rval = JSVAL_NULL;
       return JS_TRUE;
     }
 
     if (HAS_FLAGS(flags, FLAG_EXPLICIT)) {
 #ifdef DEBUG_XPCNativeWrapper
       printf("Rewrapping for deep explicit wrapper\n");
 #endif
-      if (wrappedNative == XPCNativeWrapper::SafeGetWrappedNative(obj)) {
+      if (wrappedNative == XPCNativeWrapper::GetWrappedNative(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|.  Just call XPCNativeWrapperCtor by hand
       // (passing null as the pre-created object it doesn't use anyway) so we
@@ -479,21 +422,19 @@ XPC_NW_FunctionWrapper(JSContext *cx, JS
 
   if (!obj) {
     return ThrowException(NS_ERROR_UNEXPECTED, cx);
   }
 
   // The real method we're going to call is the parent of this
   // function's JSObject.
   JSObject *methodToCallObj = STOBJ_GET_PARENT(funObj);
-  XPCWrappedNative *wrappedNative;
+  XPCWrappedNative *wrappedNative = XPCNativeWrapper::GetWrappedNative(obj);
 
-  if (!XPCNativeWrapper::GetWrappedNative(cx, obj, &wrappedNative) ||
-      !::JS_ObjectIsFunction(cx, methodToCallObj) ||
-      !wrappedNative) {
+  if (!::JS_ObjectIsFunction(cx, methodToCallObj) || !wrappedNative) {
     return ThrowException(NS_ERROR_UNEXPECTED, cx);
   }
 
   jsval v;
   if (!::JS_CallFunctionValue(cx, wrappedNative->GetFlatJSObject(),
                               OBJECT_TO_JSVAL(methodToCallObj), argc, argv,
                               &v)) {
     return JS_FALSE;
@@ -523,18 +464,17 @@ XPC_NW_GetOrSetProperty(JSContext *cx, J
       return ThrowException(NS_ERROR_UNEXPECTED, cx);
     }
   }
 
   if (!EnsureLegalActivity(cx, obj)) {
     return JS_FALSE;
   }
 
-  // Protected by EnsureLegalActivity.
-  XPCWrappedNative *wrappedNative = XPCNativeWrapper::SafeGetWrappedNative(obj);
+  XPCWrappedNative *wrappedNative = XPCNativeWrapper::GetWrappedNative(obj);
 
   if (!wrappedNative) {
     return ThrowException(NS_ERROR_INVALID_ARG, cx);
   }
 
   JSObject *nativeObj = wrappedNative->GetFlatJSObject();
 
   // We can't use XPC_NW_BYPASS here, because we need to do a full
@@ -603,18 +543,17 @@ XPC_NW_Enumerate(JSContext *cx, JSObject
   // OBJ_ENUMERATE on the XPCWrappedNative's object, called via the
   // JS_Enumerate API.  Then reflect properties named by the enumerated
   // identifiers from the wrapped native to the native wrapper.
 
   if (!EnsureLegalActivity(cx, obj)) {
     return JS_FALSE;
   }
 
-  // Protected by EnsureLegalActivity.
-  XPCWrappedNative *wn = XPCNativeWrapper::SafeGetWrappedNative(obj);
+  XPCWrappedNative *wn = XPCNativeWrapper::GetWrappedNative(obj);
   if (!wn) {
     return JS_TRUE;
   }
 
   return XPCWrapper::Enumerate(cx, obj, wn->GetFlatJSObject());
 }
 
 static JSBool
@@ -641,18 +580,17 @@ XPC_NW_NewResolve(JSContext *cx, JSObjec
 
   // We can't use XPC_NW_BYPASS here, because we need to do a full
   // OBJ_LOOKUP_PROPERTY on the wrapped native's object, in order to
   // trigger reflection along the wrapped native prototype chain.
   // All we need to do is define the property in obj if it exists in
   // the wrapped native's object.
 
   if (ShouldBypassNativeWrapper(cx, obj)) {
-    // Protected by EnsureLegalActivity.
-    XPCWrappedNative *wn = XPCNativeWrapper::SafeGetWrappedNative(obj);
+    XPCWrappedNative *wn = XPCNativeWrapper::GetWrappedNative(obj);
     if (!wn) {
       return JS_TRUE;
     }
 
     JSAutoRequest ar(cx);
 
     jsid interned_id;
     JSObject *pobj;
@@ -679,18 +617,17 @@ XPC_NW_NewResolve(JSContext *cx, JSObjec
 
   while (!XPCNativeWrapper::IsNativeWrapper(obj)) {
     obj = STOBJ_GET_PROTO(obj);
     if (!obj) {
       return ThrowException(NS_ERROR_UNEXPECTED, cx);
     }
   }
 
-  // Protected by EnsureLegalActivity.
-  XPCWrappedNative *wrappedNative = XPCNativeWrapper::SafeGetWrappedNative(obj);
+  XPCWrappedNative *wrappedNative = XPCNativeWrapper::GetWrappedNative(obj);
 
   if (!wrappedNative) {
     // No wrapped native, no properties.
 
     return JS_TRUE;
   }
 
   return XPCWrapper::ResolveNativeProperty(cx, obj,
@@ -735,18 +672,17 @@ XPC_NW_CheckAccess(JSContext *cx, JSObje
 
   // Forward to the checkObjectAccess hook in the JSContext, if any.
   JSSecurityCallbacks *callbacks = JS_GetSecurityCallbacks(cx);
   if (callbacks && callbacks->checkObjectAccess &&
       !callbacks->checkObjectAccess(cx, obj, id, mode, vp)) {
     return JS_FALSE;
   }
 
-  // This function does its own security checks.
-  XPCWrappedNative *wrappedNative = XPCNativeWrapper::SafeGetWrappedNative(obj);
+  XPCWrappedNative *wrappedNative = XPCNativeWrapper::GetWrappedNative(obj);
   if (!wrappedNative) {
     return JS_TRUE;
   }
 
   JSObject *wrapperJSObject = wrappedNative->GetFlatJSObject();
 
   JSClass *clazz = STOBJ_GET_CLASS(wrapperJSObject);
   return !clazz->checkAccess ||
@@ -781,22 +717,17 @@ XPC_NW_Construct(JSContext *cx, JSObject
   // The object given to us by the JS engine is actually a stub object (the
   // "new" object). This isn't any help to us, so instead use the function
   // object of the constructor that we're calling (which is the native
   // wrapper).
   obj = JSVAL_TO_OBJECT(argv[-2]);
 
   XPC_NW_BYPASS_TEST(cx, obj, construct, (cx, obj, argc, argv, rval));
 
-  if (!EnsureLegalActivity(cx, obj)) {
-    return JS_FALSE;
-  }
-
-  // Protected by EnsureLegalActivity.
-  XPCWrappedNative *wrappedNative = XPCNativeWrapper::SafeGetWrappedNative(obj);
+  XPCWrappedNative *wrappedNative = XPCNativeWrapper::GetWrappedNative(obj);
   if (!wrappedNative) {
     return JS_TRUE;
   }
 
   JSBool retval = JS_TRUE;
 
   if (!NATIVE_HAS_FLAG(wrappedNative, WantConstruct)) {
     return ThrowException(NS_ERROR_INVALID_ARG, cx);
@@ -866,47 +797,34 @@ XPCNativeWrapperCtor(JSContext *cx, JSOb
   jsval native = argv[0];
 
   if (JSVAL_IS_PRIMITIVE(native)) {
     return ThrowException(NS_ERROR_XPC_BAD_CONVERT_JS, cx);
   }
 
   JSObject *nativeObj = JSVAL_TO_OBJECT(native);
 
-  // Unwrap a cross origin wrapper, since we're more restrictive than it is.
-  if (STOBJ_GET_CLASS(nativeObj) == &sXPC_XOW_JSClass.base) {
-    jsval v;
-    if (!::JS_GetReservedSlot(cx, nativeObj, XPCWrapper::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);
-    }
-  } else if (STOBJ_GET_CLASS(nativeObj) == &sXPC_SJOW_JSClass.base) {
-    // Also unwrap SJOWs.
-    nativeObj = JS_GetParent(cx, nativeObj);
-    if (!nativeObj) {
-      return ThrowException(NS_ERROR_XPC_BAD_CONVERT_JS, cx);
-    }
+  // Not allowed to go from more restrictive wrappers to less restrictive
+  // wrappers.
+  if (STOBJ_GET_CLASS(nativeObj) == &sXPC_XOW_JSClass.base ||
+      STOBJ_GET_CLASS(nativeObj) == &sXPC_SJOW_JSClass.base) {
+    return ThrowException(NS_ERROR_XPC_BAD_CONVERT_JS, cx);
   }
 
   XPCWrappedNative *wrappedNative;
 
   if (XPCNativeWrapper::IsNativeWrapper(nativeObj)) {
     // We're asked to wrap an already wrapped object. Re-wrap the
     // object wrapped by the given wrapper.
 
 #ifdef DEBUG_XPCNativeWrapper
     printf("Wrapping already wrapped object\n");
 #endif
 
-    // It's always safe to re-wrap an object.
-    wrappedNative = XPCNativeWrapper::SafeGetWrappedNative(nativeObj);
+    wrappedNative = XPCNativeWrapper::GetWrappedNative(nativeObj);
 
     if (!wrappedNative) {
       return ThrowException(NS_ERROR_INVALID_ARG, cx);
     }
 
     nativeObj = wrappedNative->GetFlatJSObject();
     native = OBJECT_TO_JSVAL(nativeObj);
   } else {
@@ -1040,44 +958,38 @@ XPCNativeWrapperCtor(JSContext *cx, JSOb
   }
 
   return JS_TRUE;
 }
 
 static void
 XPC_NW_Trace(JSTracer *trc, JSObject *obj)
 {
-  // Untrusted code can't trigger this.
-  XPCWrappedNative *wrappedNative = XPCNativeWrapper::SafeGetWrappedNative(obj);
+  XPCWrappedNative *wrappedNative = XPCNativeWrapper::GetWrappedNative(obj);
 
   if (wrappedNative && wrappedNative->IsValid()) {
     JS_CALL_OBJECT_TRACER(trc, wrappedNative->GetFlatJSObject(),
                           "wrappedNative.flatJSObject");
   }
 }
 
 static JSBool
 XPC_NW_Equality(JSContext *cx, JSObject *obj, jsval v, JSBool *bp)
 {
   NS_ASSERTION(XPCNativeWrapper::IsNativeWrapper(obj),
                "Uh, we should only ever be called for XPCNativeWrapper "
                "objects!");
 
-  if (!EnsureLegalActivity(cx, obj)) {
-    return JS_FALSE;
-  }
-
   if (JSVAL_IS_PRIMITIVE(v)) {
     *bp = JS_FALSE;
 
     return JS_TRUE;
   }
 
-  // Protected by EnsureLegalActivity.
-  XPCWrappedNative *wrappedNative = XPCNativeWrapper::SafeGetWrappedNative(obj);
+  XPCWrappedNative *wrappedNative = XPCNativeWrapper::GetWrappedNative(obj);
 
   if (wrappedNative && wrappedNative->IsValid() &&
       NATIVE_HAS_FLAG(wrappedNative, WantEquality)) {
     // Forward the call to the wrapped native's Equality() hook.
     nsresult rv = wrappedNative->GetScriptableCallback()->
       Equality(wrappedNative, cx, obj, v, bp);
 
     if (NS_FAILED(rv)) {
@@ -1103,18 +1015,17 @@ XPC_NW_toString(JSContext *cx, JSObject 
       return ThrowException(NS_ERROR_UNEXPECTED, cx);
     }
   }
 
   if (!EnsureLegalActivity(cx, obj)) {
     return JS_FALSE;
   }
 
-  // Protected by EnsureLegalActivity.
-  XPCWrappedNative *wrappedNative = XPCNativeWrapper::SafeGetWrappedNative(obj);
+  XPCWrappedNative *wrappedNative = XPCNativeWrapper::GetWrappedNative(obj);
 
   if (!wrappedNative) {
     // toString() called on XPCNativeWrapper.prototype
     NS_NAMED_LITERAL_STRING(protoString, "[object XPCNativeWrapper]");
     JSString *str =
       ::JS_NewUCStringCopyN(cx, reinterpret_cast<const jschar*>
                                                 (protoString.get()),
                             protoString.Length());
--- a/js/src/xpconnect/src/XPCNativeWrapper.h
+++ b/js/src/xpconnect/src/XPCNativeWrapper.h
@@ -56,26 +56,21 @@ public:
     return clazz == &sXPC_NW_JSClass.base;
   }
 
   static PRBool IsNativeWrapper(JSObject *obj)
   {
     return STOBJ_GET_CLASS(obj) == &sXPC_NW_JSClass.base;
   }
 
-  static JSBool GetWrappedNative(JSContext *cx, JSObject *obj,
-                                 XPCWrappedNative **aWrappedNative);
-
-  // NB: Use the following carefully.
-  static XPCWrappedNative *SafeGetWrappedNative(JSObject *obj)
+  static XPCWrappedNative *GetWrappedNative(JSObject *obj)
   {
-      return static_cast<XPCWrappedNative *>(xpc_GetJSPrivate(obj));
+    return (XPCWrappedNative *)xpc_GetJSPrivate(obj);
   }
 
-
   static JSClass *GetJSClass()
   {
     return &sXPC_NW_JSClass.base;
   }
 
   static void ClearWrappedNativeScopes(JSContext* cx,
                                        XPCWrappedNative* wrapper);
 
--- a/js/src/xpconnect/src/xpcquickstubs.cpp
+++ b/js/src/xpconnect/src/xpcquickstubs.cpp
@@ -530,18 +530,18 @@ xpc_qsUnwrapThisImpl(JSContext *cx,
             }
 
             // This goto is a bug, dutifully copied from
             // XPCWrappedNative::GetWrappedNativeOfJSObject.
             goto next;
         }
         else if(XPCNativeWrapper::IsNativeWrapperClass(clazz))
         {
-            if(!XPCNativeWrapper::GetWrappedNative(cx, cur, &wrapper) ||
-               !wrapper)
+            wrapper = XPCNativeWrapper::GetWrappedNative(cur);
+            if(!wrapper)
                 goto next;
         }
         else if(IsXPCSafeJSObjectWrapperClass(clazz))
         {
             cur = STOBJ_GET_PARENT(cur);
             NS_ASSERTION(cur, "SJOW wrapping nothing");
             continue;
         }
--- a/js/src/xpconnect/src/xpcwrappednative.cpp
+++ b/js/src/xpconnect/src/xpcwrappednative.cpp
@@ -1439,18 +1439,18 @@ return_tearoff:
         JSObject *unsafeObj;
         if(clazz == &sXPC_XOW_JSClass.base &&
            (unsafeObj = XPCWrapper::Unwrap(cx, cur)))
             return GetWrappedNativeOfJSObject(cx, unsafeObj, funobj, pobj2,
                                               pTearOff);
 
         if(XPCNativeWrapper::IsNativeWrapperClass(clazz))
         {
-            XPCWrappedNative* wrapper;
-            if(XPCNativeWrapper::GetWrappedNative(cx, cur, &wrapper) && wrapper)
+            XPCWrappedNative* wrapper = XPCNativeWrapper::GetWrappedNative(cur);
+            if(wrapper)
                 return GetWrappedNativeOfJSObject(cx, wrapper->GetFlatJSObject(),
                                                   funobj, pobj2, pTearOff);
         }
 
         if(IsXPCSafeJSObjectWrapperClass(clazz) &&
            (unsafeObj = STOBJ_GET_PARENT(cur)))
             return GetWrappedNativeOfJSObject(cx, unsafeObj, funobj, pobj2,
                                               pTearOff);
--- a/js/src/xpconnect/src/xpcwrappednativejsops.cpp
+++ b/js/src/xpconnect/src/xpcwrappednativejsops.cpp
@@ -731,19 +731,17 @@ XPC_WN_NoHelper_Resolve(JSContext *cx, J
 }
 
 nsISupports *
 XPC_GetIdentityObject(JSContext *cx, JSObject *obj)
 {
     XPCWrappedNative *wrapper;
 
     if(XPCNativeWrapper::IsNativeWrapper(obj))
-        // Note: It's okay to use SafeGetWrappedNative here since we only do
-        // identity checking on the returned object.
-        wrapper = XPCNativeWrapper::SafeGetWrappedNative(obj);
+        wrapper = XPCNativeWrapper::GetWrappedNative(obj);
     else
         wrapper = XPCWrappedNative::GetWrappedNativeOfJSObject(cx, obj);
 
     if(!wrapper) {
         JSObject *unsafeObj = XPC_SJOW_GetUnsafeObject(obj);
         if(unsafeObj)
             return XPC_GetIdentityObject(cx, unsafeObj);
 
--- a/js/src/xpconnect/tests/mochitest/test_wrappers.html
+++ b/js/src/xpconnect/tests/mochitest/test_wrappers.html
@@ -47,18 +47,16 @@
     ok(!(new XPCSafeJSObjectWrapper({}) == new XPCSafeJSObjectWrapper({})),
        'SJOWs equality hook returns false correctly');
 
     let (obj = {}) {
         ok(new XPCSafeJSObjectWrapper(obj) == new XPCSafeJSObjectWrapper(obj),
            'SJOWs equality hook returns true correctly');
     }
 
-    ok(new XPCSafeJSObjectWrapper(window) == new XPCNativeWrapper(window),
-       'SJOWs equality hook returns true correctly against XPCNW');
     ok(new XPCSafeJSObjectWrapper(window) == window,
        'SJOWs equality hook returns true correctly against XOW');
 
     is(typeof(new XPCSafeJSObjectWrapper(function(){})), 'function',
        'SJOWs look like functions when they wrap functions');
     is(typeof(new XPCSafeJSObjectWrapper({})), 'object',
        'SJOWs look like objects when they wrap objects');