Bug 478438 - Can't access allAccess properties of cross-origin XPCNativeWrappers. r+sr=mrbkap

js/src/xpconnect/src/XPCCrossOriginWrapper.cpp

@@ -439,11 +439,13 @@ XPC_XOW_WrapFunction(JSContext *cx, JSObject *outerObj, JSObject *funobj,
   }
 
   JSObject *funWrapperObj = JS_GetFunctionObject(funWrapper);
-  if (!JS_SetReservedSlot(cx, funWrapperObj, 0, funobjVal)) {
+  *rval = OBJECT_TO_JSVAL(funWrapperObj);
+
+  if (!JS_SetReservedSlot(cx, funWrapperObj, XPCWrapper::eXOWWrappedFunctionSlot, funobjVal) ||
+      !JS_SetReservedSlot(cx, funWrapperObj, XPCWrapper::eAllAccessSlot, JSVAL_FALSE)) {
     return JS_FALSE;
   }
 
-  *rval = OBJECT_TO_JSVAL(funWrapperObj);
   return JS_TRUE;
 }
 
@@ -676,13 +678,10 @@ XPC_XOW_GetOrSetProperty(JSContext *cx, JSObject *obj, jsval id, jsval *vp,
       if (!ssm) {
         return ThrowException(NS_ERROR_NOT_INITIALIZED, cx);
       }
-
-      PRUint32 check = isSet
-                       ? (PRUint32)nsIXPCSecurityManager::ACCESS_SET_PROPERTY
-                       : (PRUint32)nsIXPCSecurityManager::ACCESS_GET_PROPERTY;
       rv = ssm->CheckPropertyAccess(cx, wrappedObj,
                                     STOBJ_GET_CLASS(wrappedObj)->name,
-                                    id, check);
+                                    id, isSet ? XPCWrapper::sSecMgrSetProp
+                                              : XPCWrapper::sSecMgrGetProp);
       if (NS_FAILED(rv)) {
         // The security manager threw an exception for us.
         return JS_FALSE;

js/src/xpconnect/src/XPCNativeWrapper.cpp

@@ -193,7 +193,8 @@ ThrowException(nsresult ex, JSContext *cx)
 
 static inline
 JSBool
-EnsureLegalActivity(JSContext *cx, JSObject *obj)
+EnsureLegalActivity(JSContext *cx, JSObject *obj,
+                    jsval id = JSVAL_VOID, PRUint32 accessType = 0)
 {
   nsIScriptSecurityManager *ssm = XPCWrapper::GetSecurityManager();
   if (!ssm) {
@@ -227,6 +228,18 @@ EnsureLegalActivity(JSContext *cx, JSObject *obj)
     PRBool subsumes;
     if (NS_FAILED(subjectPrincipal->Subsumes(objectPrincipal, &subsumes)) ||
         !subsumes) {
+
+      JSObject* flatObj;
+      if (!JSVAL_IS_VOID(id) &&
+          (accessType & (XPCWrapper::sSecMgrSetProp |
+                         XPCWrapper::sSecMgrGetProp)) &&
+          (flatObj = wn->GetFlatJSObject())) {
+        rv = ssm->CheckPropertyAccess(cx, flatObj,
+                                      STOBJ_GET_CLASS(flatObj)->name,
+                                      id, accessType);
+        return NS_SUCCEEDED(rv);
+      }
+
       return ThrowException(NS_ERROR_XPC_SECURITY_MANAGER_VETO, cx);
     }
   }
@@ -336,6 +349,9 @@ XPC_NW_WrapFunction(JSContext* cx, JSObject* funobj, jsval *rval)
   JSObject* funWrapperObj = ::JS_GetFunctionObject(funWrapper);
   ::JS_SetParent(cx, funWrapperObj, funobj);
   *rval = OBJECT_TO_JSVAL(funWrapperObj);
+
+  JS_SetReservedSlot(cx, funWrapperObj, XPCWrapper::eAllAccessSlot, JSVAL_FALSE);
+
   return JS_TRUE;
 }
 
@@ -363,13 +379,17 @@ XPC_NW_AddProperty(JSContext *cx, JSObject *obj, jsval id, jsval *vp)
 
   jsval flags;
   ::JS_GetReservedSlot(cx, obj, 0, &flags);
+  // The purpose of XPC_NW_AddProperty is to wrap any object set on the
+  // XPCNativeWrapper by the wrapped object's scriptable helper, so bail
+  // here if the scriptable helper is not currently adding a property.
+  // See comment above #define FLAG_RESOLVING in XPCWrapper.h.
   if (!HAS_FLAGS(flags, FLAG_RESOLVING)) {
     return JS_TRUE;
   }
 
   // Note: no need to protect *vp from GC here, since it's already in the slot
   // on |obj|.
-  return EnsureLegalActivity(cx, obj) &&
+  return EnsureLegalActivity(cx, obj, id, XPCWrapper::sSecMgrSetProp) &&
          XPC_NW_RewrapIfDeepWrapper(cx, obj, *vp, vp);
 }
 
@@ -498,11 +518,19 @@ XPC_NW_FunctionWrapper(JSContext *cx, JSObject *obj, uintN argc, jsval *argv,
   // 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 = nsnull;
 
-  if (!XPCNativeWrapper::GetWrappedNative(cx, obj, &wrappedNative) ||
-      !::JS_ObjectIsFunction(cx, methodToCallObj) ||
-      !wrappedNative) {
+  jsval isAllAccess;
+  if (::JS_GetReservedSlot(cx, funObj,
+                           XPCWrapper::eAllAccessSlot,
+                           &isAllAccess) &&
+      JSVAL_TO_BOOLEAN(isAllAccess)) {
+    wrappedNative = XPCNativeWrapper::SafeGetWrappedNative(obj);
+  } else if (!XPCNativeWrapper::GetWrappedNative(cx, obj, &wrappedNative)) {
+    wrappedNative = nsnull;
+  }
+
+  if (!wrappedNative || !::JS_ObjectIsFunction(cx, methodToCallObj)) {
     return ThrowException(NS_ERROR_UNEXPECTED, cx);
   }
 
@@ -538,7 +566,9 @@ XPC_NW_GetOrSetProperty(JSContext *cx, JSObject *obj, jsval id, jsval *vp,
     }
   }
 
-  if (!EnsureLegalActivity(cx, obj)) {
+  if (!EnsureLegalActivity(cx, obj, id,
+                           aIsSet ? XPCWrapper::sSecMgrSetProp
+                                  : XPCWrapper::sSecMgrGetProp)) {
     return JS_FALSE;
   }
 
@@ -661,7 +691,10 @@ XPC_NW_NewResolve(JSContext *cx, JSObject *obj, jsval id, uintN flags,
                              nsnull, nsnull, 0);
   }
 
-  if (!EnsureLegalActivity(cx, obj)) {
+  PRUint32 accessType =
+    (flags & JSRESOLVE_ASSIGNING) ? XPCWrapper::sSecMgrSetProp
+                                  : XPCWrapper::sSecMgrGetProp;
+  if (!EnsureLegalActivity(cx, obj, id, accessType)) {
     return JS_FALSE;
   }
 

js/src/xpconnect/src/XPCWrapper.cpp

@@ -55,6 +55,11 @@ XPCWrapper::sNumSlots = 2;
 JSNative
 XPCWrapper::sEvalNative = nsnull;
 
+const PRUint32
+XPCWrapper::sSecMgrSetProp = nsIXPCSecurityManager::ACCESS_SET_PROPERTY;
+const PRUint32
+XPCWrapper::sSecMgrGetProp = nsIXPCSecurityManager::ACCESS_GET_PROPERTY;
+
 static void
 IteratorFinalize(JSContext *cx, JSObject *obj)
 {
@@ -551,6 +556,12 @@ XPCWrapper::ResolveNativeProperty(JSContext *cx, JSObject *wrapperObj,
                       isNativeWrapper)) {
       return JS_FALSE;
     }
+
+    // Since the XPC_*_NewResolve functions ensure that the method's property
+    // name is accessible, we set the eAllAccessSlot bit, which indicates to
+    // XPC_NW_FunctionWrapper that the method is safe to unwrap and call, even
+    // if XPCNativeWrapper::GetWrappedNative disagrees.
+    JS_SetReservedSlot(cx, JSVAL_TO_OBJECT(v), eAllAccessSlot, JSVAL_TRUE);
   }
 
   // Make sure v doesn't go away while we mess with it.

js/src/xpconnect/src/XPCWrapper.h

@@ -142,6 +142,14 @@ public:
    */
   static JSNative sEvalNative;
 
+  typedef enum FunctionObjectSlot {
+    eXOWWrappedFunctionSlot = 0,
+    eAllAccessSlot = 1
+  };
+
+  // Helpful for keeping lines short:
+  static const PRUint32 sSecMgrSetProp, sSecMgrGetProp;
+
   /**
    * Given a context and a global object, fill our eval native.
    */

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

@@ -54,6 +54,7 @@ _TEST_FILES =	inner.html \
 		test_wrappers.html \
 		test_bug446584.html \
 		test_bug462428.html \
+		test_bug478438.html \
 		$(NULL)
 
 libs:: $(_TEST_FILES)

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

@@ -0,0 +1,66 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=478438
+-->
+<head>
+  <title>Test for Bug 478438</title>
+  <script type="application/javascript" src="/MochiKit/MochiKit.js"></script>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+  <script type="application/javascript;version=1.8">
+    function fail(s) { ok(false, s) }
+    function pass(s) { ok(true, s) }
+    (pass.opposite = fail).opposite = pass;
+
+    function test() {
+      if (test.calledAlready)
+        return;
+      test.calledAlready = true;
+
+      var iwin = (new XPCNativeWrapper(document)).getElementById("f").contentWindow;
+
+      function testOne(fn, onAllow, infinitive) {
+        try { fn(); onAllow("able " + infinitive) }
+        catch (e) { onAllow.opposite("unable " + infinitive + ": " + e) }
+      }
+
+      testOne(function() iwin.focus, pass,
+              "to resolve/get allAccess property iwin.focus");
+
+      testOne(function() iwin.focus(), pass,
+              "to call allAccess method iwin.focus");
+
+      testOne(function() iwin.alert, fail,
+              "to resolve/get restricted property iwin.alert");
+
+      testOne(function() iwin.alert(), fail,
+              "to call restricted method iwin.alert");
+
+      testOne(function() iwin.location.toString(), fail,
+              "to call restricted method iwin.location.toString");
+
+      testOne(function() { iwin.location = "http://example.org" }, pass,
+              "to set writable property iwin.location");
+
+      SimpleTest.finish();
+    }
+  </script>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=478438">Mozilla Bug 478438</a>
+<p id="display"></p>
+<div id="content">
+  <iframe id="f" src="http://example.com" onload="test()"></iframe>
+</div>
+<pre id="test">
+<script type="application/javascript">
+
+/** Test for Bug 478438 **/
+
+SimpleTest.waitForExplicitFinish();
+
+</script>
+</pre>
+</body>
+</html>