Bug 794943 - Remove CheckXPCPermissions. r=mrbkap
authorBobby Holley <bobbyholley@gmail.com>
Thu, 16 Jan 2014 15:45:40 -0800
changeset 163895 5379a6ae2dcc89bbb1808cb09f92ceb4edeb6a1b
parent 163894 8b5c0de7c8c5e108d61193f1f56ee9436ab918bd
child 163896 13a3ebc3611711b907e6533a5133375d90bd08d1
push idunknown
push userunknown
push dateunknown
reviewersmrbkap
bugs794943
milestone29.0a1
Bug 794943 - Remove CheckXPCPermissions. r=mrbkap
caps/include/nsScriptSecurityManager.h
caps/src/nsScriptSecurityManager.cpp
--- a/caps/include/nsScriptSecurityManager.h
+++ b/caps/include/nsScriptSecurityManager.h
@@ -157,44 +157,16 @@ private:
                             nsIPrincipal** result);
 
     // Returns null if a principal cannot be found.  Note that rv can be NS_OK
     // when this happens -- this means that there was no script for the
     // context.  Callers MUST pass in a non-null rv here.
     nsIPrincipal*
     GetSubjectPrincipal(JSContext* cx, nsresult* rv);
 
-    /**
-     * Check capability levels for an |aObj| that implements
-     * nsISecurityCheckedComponent.
-     *
-     * NB: This function also checks to see if aObj is a plugin and the user
-     * has set the "security.xpconnect.plugin.unrestricted" pref to allow
-     * anybody to script plugin objects from anywhere.
-     *
-     * @param cx The context we're running on.
-     *           NB: If null, "sameOrigin" does not have any effect.
-     * @param aObj The nsISupports representation of the object in question
-     *             object, possibly null.
-     * @param aJSObject The JSObject representation of the object in question
-     *                  if |cx| is non-null and |aObjectSecurityLevel| is
-     *                  "sameOrigin". If null will be calculated from aObj (if
-     *                  non-null) if and only if aObj is an XPCWrappedJS. The
-     *                  rationale behind this is that if we're creating a JS
-     *                  wrapper for an XPCWrappedJS, this object definitely
-     *                  expects to be exposed to JS.
-     * @param aSubjectPrincipal The nominal subject principal used when
-     *                          aObjectSecurityLevel is "sameOrigin". If null,
-     *                          this is calculated if it's needed.
-     */
-    nsresult
-    CheckXPCPermissions(JSContext* cx,
-                        nsISupports* aObj, JSObject* aJSObject,
-                        nsIPrincipal* aSubjectPrincipal);
-
     nsresult
     Init();
 
     nsresult
     InitPrefs();
 
     inline void
     ScriptSecurityPrefChanged();
--- a/caps/src/nsScriptSecurityManager.cpp
+++ b/caps/src/nsScriptSecurityManager.cpp
@@ -602,132 +602,133 @@ nsScriptSecurityManager::CheckPropertyAc
                                         aAction);
     }
 
     if (NS_SUCCEEDED(rv))
     {
         return rv;
     }
 
-    rv = CheckXPCPermissions(cx, aObj, jsObject, subjectPrincipal);
 
-    if (NS_FAILED(rv)) //-- Security tests failed, access is denied, report error
+    if (SubjectIsPrivileged())
+    {
+        return NS_OK;
+    }
+
+    //-- Security tests failed, access is denied, report error
+    nsAutoString stringName;
+    switch(aAction)
     {
-        nsAutoString stringName;
-        switch(aAction)
-        {
-        case nsIXPCSecurityManager::ACCESS_GET_PROPERTY:
-            stringName.AssignLiteral("GetPropertyDeniedOrigins");
-            break;
-        case nsIXPCSecurityManager::ACCESS_SET_PROPERTY:
-            stringName.AssignLiteral("SetPropertyDeniedOrigins");
-            break;
-        case nsIXPCSecurityManager::ACCESS_CALL_METHOD:
-            stringName.AssignLiteral("CallMethodDeniedOrigins");
+    case nsIXPCSecurityManager::ACCESS_GET_PROPERTY:
+        stringName.AssignLiteral("GetPropertyDeniedOrigins");
+        break;
+    case nsIXPCSecurityManager::ACCESS_SET_PROPERTY:
+        stringName.AssignLiteral("SetPropertyDeniedOrigins");
+        break;
+    case nsIXPCSecurityManager::ACCESS_CALL_METHOD:
+        stringName.AssignLiteral("CallMethodDeniedOrigins");
+    }
+
+    // Null out objectPrincipal for now, so we don't leak information about
+    // it.  Whenever we can report different error strings to content and
+    // the UI we can take this out again.
+    objectPrincipal = nullptr;
+
+    NS_ConvertUTF8toUTF16 classInfoName(classInfoData.GetName());
+    nsAutoCString subjectOrigin;
+    nsAutoCString subjectDomain;
+    if (!nsAutoInPrincipalDomainOriginSetter::sInPrincipalDomainOrigin) {
+        nsCOMPtr<nsIURI> uri, domain;
+        subjectPrincipal->GetURI(getter_AddRefs(uri));
+        if (uri) { // Object principal might be expanded
+            GetOriginFromURI(uri, subjectOrigin);
         }
-
-        // Null out objectPrincipal for now, so we don't leak information about
-        // it.  Whenever we can report different error strings to content and
-        // the UI we can take this out again.
-        objectPrincipal = nullptr;
+        subjectPrincipal->GetDomain(getter_AddRefs(domain));
+        if (domain) {
+            GetOriginFromURI(domain, subjectDomain);
+        }
+    } else {
+        subjectOrigin.AssignLiteral("the security manager");
+    }
+    NS_ConvertUTF8toUTF16 subjectOriginUnicode(subjectOrigin);
+    NS_ConvertUTF8toUTF16 subjectDomainUnicode(subjectDomain);
 
-        NS_ConvertUTF8toUTF16 className(classInfoData.GetName());
-        nsAutoCString subjectOrigin;
-        nsAutoCString subjectDomain;
-        if (!nsAutoInPrincipalDomainOriginSetter::sInPrincipalDomainOrigin) {
-            nsCOMPtr<nsIURI> uri, domain;
-            subjectPrincipal->GetURI(getter_AddRefs(uri));
-            if (uri) { // Object principal might be expanded
-                GetOriginFromURI(uri, subjectOrigin);
-            }
-            subjectPrincipal->GetDomain(getter_AddRefs(domain));
-            if (domain) {
-                GetOriginFromURI(domain, subjectDomain);
-            }
-        } else {
-            subjectOrigin.AssignLiteral("the security manager");
+    nsAutoCString objectOrigin;
+    nsAutoCString objectDomain;
+    if (!nsAutoInPrincipalDomainOriginSetter::sInPrincipalDomainOrigin &&
+        objectPrincipal) {
+        nsCOMPtr<nsIURI> uri, domain;
+        objectPrincipal->GetURI(getter_AddRefs(uri));
+        if (uri) { // Object principal might be system
+            GetOriginFromURI(uri, objectOrigin);
+        }
+        objectPrincipal->GetDomain(getter_AddRefs(domain));
+        if (domain) {
+            GetOriginFromURI(domain, objectDomain);
         }
-        NS_ConvertUTF8toUTF16 subjectOriginUnicode(subjectOrigin);
-        NS_ConvertUTF8toUTF16 subjectDomainUnicode(subjectDomain);
+    }
+    NS_ConvertUTF8toUTF16 objectOriginUnicode(objectOrigin);
+    NS_ConvertUTF8toUTF16 objectDomainUnicode(objectDomain);
+
+    nsXPIDLString errorMsg;
+    const char16_t *formatStrings[] =
+    {
+        subjectOriginUnicode.get(),
+        classInfoName.get(),
+        IDToString(cx, property),
+        objectOriginUnicode.get(),
+        subjectDomainUnicode.get(),
+        objectDomainUnicode.get()
+    };
+
+    uint32_t length = ArrayLength(formatStrings);
 
-        nsAutoCString objectOrigin;
-        nsAutoCString objectDomain;
-        if (!nsAutoInPrincipalDomainOriginSetter::sInPrincipalDomainOrigin &&
-            objectPrincipal) {
-            nsCOMPtr<nsIURI> uri, domain;
-            objectPrincipal->GetURI(getter_AddRefs(uri));
-            if (uri) { // Object principal might be system
-                GetOriginFromURI(uri, objectOrigin);
-            }
-            objectPrincipal->GetDomain(getter_AddRefs(domain));
-            if (domain) {
-                GetOriginFromURI(domain, objectDomain);
+    // XXXbz Our localization system is stupid and can't handle not showing
+    // some strings that get passed in.  Which means that we have to get
+    // our length precisely right: it has to be exactly the number of
+    // strings our format string wants.  This means we'll have to move
+    // strings in the array as needed, sadly...
+    if (nsAutoInPrincipalDomainOriginSetter::sInPrincipalDomainOrigin ||
+        !objectPrincipal) {
+        stringName.AppendLiteral("OnlySubject");
+        length -= 3;
+    } else {
+        // default to a length that doesn't include the domains, then
+        // increase it as needed.
+        length -= 2;
+        if (!subjectDomainUnicode.IsEmpty()) {
+            stringName.AppendLiteral("SubjectDomain");
+            length += 1;
+        }
+        if (!objectDomainUnicode.IsEmpty()) {
+            stringName.AppendLiteral("ObjectDomain");
+            length += 1;
+            if (length != ArrayLength(formatStrings)) {
+                // We have an object domain but not a subject domain.
+                // Scoot our string over one slot.  See the XXX comment
+                // above for why we need to do this.
+                formatStrings[length-1] = formatStrings[length];
             }
         }
-        NS_ConvertUTF8toUTF16 objectOriginUnicode(objectOrigin);
-        NS_ConvertUTF8toUTF16 objectDomainUnicode(objectDomain);
-
-        nsXPIDLString errorMsg;
-        const char16_t *formatStrings[] =
-        {
-            subjectOriginUnicode.get(),
-            className.get(),
-            IDToString(cx, property),
-            objectOriginUnicode.get(),
-            subjectDomainUnicode.get(),
-            objectDomainUnicode.get()
-        };
-
-        uint32_t length = ArrayLength(formatStrings);
-
-        // XXXbz Our localization system is stupid and can't handle not showing
-        // some strings that get passed in.  Which means that we have to get
-        // our length precisely right: it has to be exactly the number of
-        // strings our format string wants.  This means we'll have to move
-        // strings in the array as needed, sadly...
-        if (nsAutoInPrincipalDomainOriginSetter::sInPrincipalDomainOrigin ||
-            !objectPrincipal) {
-            stringName.AppendLiteral("OnlySubject");
-            length -= 3;
-        } else {
-            // default to a length that doesn't include the domains, then
-            // increase it as needed.
-            length -= 2;
-            if (!subjectDomainUnicode.IsEmpty()) {
-                stringName.AppendLiteral("SubjectDomain");
-                length += 1;
-            }
-            if (!objectDomainUnicode.IsEmpty()) {
-                stringName.AppendLiteral("ObjectDomain");
-                length += 1;
-                if (length != ArrayLength(formatStrings)) {
-                    // We have an object domain but not a subject domain.
-                    // Scoot our string over one slot.  See the XXX comment
-                    // above for why we need to do this.
-                    formatStrings[length-1] = formatStrings[length];
-                }
-            }
-        }
-        
-        // We need to keep our existing failure rv and not override it
-        // with a likely success code from the following string bundle
-        // call in order to throw the correct security exception later.
-        nsresult rv2 = sStrBundle->FormatStringFromName(stringName.get(),
-                                                        formatStrings,
-                                                        length,
-                                                        getter_Copies(errorMsg));
-        if (NS_FAILED(rv2)) {
-            // Might just be missing the string...  Do our best
-            errorMsg = stringName;
-        }
-
-        SetPendingException(cx, errorMsg.get());
+    }
+    
+    // We need to keep our existing failure rv and not override it
+    // with a likely success code from the following string bundle
+    // call in order to throw the correct security exception later.
+    nsresult rv2 = sStrBundle->FormatStringFromName(stringName.get(),
+                                                    formatStrings,
+                                                    length,
+                                                    getter_Copies(errorMsg));
+    if (NS_FAILED(rv2)) {
+        // Might just be missing the string...  Do our best
+        errorMsg = stringName;
     }
 
-    return rv;
+    SetPendingException(cx, errorMsg.get());
+    return NS_ERROR_DOM_XPCONNECT_ACCESS_DENIED;
 }
 
 /* static */
 nsresult
 nsScriptSecurityManager::CheckSameOriginPrincipal(nsIPrincipal* aSubject,
                                                   nsIPrincipal* aObject)
 {
     /*
@@ -1488,88 +1489,87 @@ nsScriptSecurityManager::CanCreateWrappe
     }
 
     // We give remote-XUL whitelisted domains a free pass here. See bug 932906.
     if (!xpc::AllowXBLScope(js::GetContextCompartment(cx)))
     {
         return NS_OK;
     }
 
-    nsresult rv = CheckXPCPermissions(cx, aObj, nullptr, nullptr);
-    if (NS_FAILED(rv))
+    if (SubjectIsPrivileged())
     {
-        //-- Access denied, report an error
-        NS_ConvertUTF8toUTF16 strName("CreateWrapperDenied");
-        nsAutoCString origin;
-        nsresult rv2;
-        nsIPrincipal* subjectPrincipal = doGetSubjectPrincipal(&rv2);
-        if (NS_SUCCEEDED(rv2) && subjectPrincipal) {
-            GetPrincipalDomainOrigin(subjectPrincipal, origin);
-        }
-        NS_ConvertUTF8toUTF16 originUnicode(origin);
-        NS_ConvertUTF8toUTF16 className(objClassInfo.GetName());
-        const char16_t* formatStrings[] = {
-            className.get(),
-            originUnicode.get()
-        };
-        uint32_t length = ArrayLength(formatStrings);
-        if (originUnicode.IsEmpty()) {
-            --length;
-        } else {
-            strName.AppendLiteral("ForOrigin");
-        }
-        nsXPIDLString errorMsg;
-        // We need to keep our existing failure rv and not override it
-        // with a likely success code from the following string bundle
-        // call in order to throw the correct security exception later.
-        rv2 = sStrBundle->FormatStringFromName(strName.get(),
-                                               formatStrings,
-                                               length,
-                                               getter_Copies(errorMsg));
-        NS_ENSURE_SUCCESS(rv2, rv2);
-
-        SetPendingException(cx, errorMsg.get());
+        return NS_OK;
     }
 
-    return rv;
+    //-- Access denied, report an error
+    NS_ConvertUTF8toUTF16 strName("CreateWrapperDenied");
+    nsAutoCString origin;
+    nsresult rv2;
+    nsIPrincipal* subjectPrincipal = doGetSubjectPrincipal(&rv2);
+    if (NS_SUCCEEDED(rv2) && subjectPrincipal) {
+        GetPrincipalDomainOrigin(subjectPrincipal, origin);
+    }
+    NS_ConvertUTF8toUTF16 originUnicode(origin);
+    NS_ConvertUTF8toUTF16 classInfoName(objClassInfo.GetName());
+    const char16_t* formatStrings[] = {
+        classInfoName.get(),
+        originUnicode.get()
+    };
+    uint32_t length = ArrayLength(formatStrings);
+    if (originUnicode.IsEmpty()) {
+        --length;
+    } else {
+        strName.AppendLiteral("ForOrigin");
+    }
+    nsXPIDLString errorMsg;
+    // We need to keep our existing failure rv and not override it
+    // with a likely success code from the following string bundle
+    // call in order to throw the correct security exception later.
+    rv2 = sStrBundle->FormatStringFromName(strName.get(),
+                                           formatStrings,
+                                           length,
+                                           getter_Copies(errorMsg));
+    NS_ENSURE_SUCCESS(rv2, rv2);
+
+    SetPendingException(cx, errorMsg.get());
+    return NS_ERROR_DOM_XPCONNECT_ACCESS_DENIED;
 }
 
 NS_IMETHODIMP
 nsScriptSecurityManager::CanCreateInstance(JSContext *cx,
                                            const nsCID &aCID)
 {
-    nsresult rv = CheckXPCPermissions(cx, nullptr, nullptr, nullptr);
-    if (NS_FAILED(rv))
-    {
-        //-- Access denied, report an error
-        nsAutoCString errorMsg("Permission denied to create instance of class. CID=");
-        char cidStr[NSID_LENGTH];
-        aCID.ToProvidedString(cidStr);
-        errorMsg.Append(cidStr);
-        SetPendingException(cx, errorMsg.get());
+    if (SubjectIsPrivileged()) {
+        return NS_OK;
     }
-    return rv;
+
+    //-- Access denied, report an error
+    nsAutoCString errorMsg("Permission denied to create instance of class. CID=");
+    char cidStr[NSID_LENGTH];
+    aCID.ToProvidedString(cidStr);
+    errorMsg.Append(cidStr);
+    SetPendingException(cx, errorMsg.get());
+    return NS_ERROR_DOM_XPCONNECT_ACCESS_DENIED;
 }
 
 NS_IMETHODIMP
 nsScriptSecurityManager::CanGetService(JSContext *cx,
                                        const nsCID &aCID)
 {
-    nsresult rv = CheckXPCPermissions(cx, nullptr, nullptr, nullptr);
-    if (NS_FAILED(rv))
-    {
-        //-- Access denied, report an error
-        nsAutoCString errorMsg("Permission denied to get service. CID=");
-        char cidStr[NSID_LENGTH];
-        aCID.ToProvidedString(cidStr);
-        errorMsg.Append(cidStr);
-        SetPendingException(cx, errorMsg.get());
+    if (SubjectIsPrivileged()) {
+        return NS_OK;
     }
 
-    return rv;
+    //-- Access denied, report an error
+    nsAutoCString errorMsg("Permission denied to get service. CID=");
+    char cidStr[NSID_LENGTH];
+    aCID.ToProvidedString(cidStr);
+    errorMsg.Append(cidStr);
+    SetPendingException(cx, errorMsg.get());
+    return NS_ERROR_DOM_XPCONNECT_ACCESS_DENIED;
 }
 
 
 NS_IMETHODIMP
 nsScriptSecurityManager::CanAccess(uint32_t aAction,
                                    nsAXPCNativeCallContext* aCallContext,
                                    JSContext* cx,
                                    JSObject* aJSObject,
@@ -1577,31 +1577,16 @@ nsScriptSecurityManager::CanAccess(uint3
                                    nsIClassInfo* aClassInfo,
                                    jsid aPropertyName)
 {
     return CheckPropertyAccessImpl(aAction, aCallContext, cx,
                                    aJSObject, aObj, aClassInfo,
                                    nullptr, aPropertyName);
 }
 
-nsresult
-nsScriptSecurityManager::CheckXPCPermissions(JSContext* cx,
-                                             nsISupports* aObj, JSObject* aJSObject,
-                                             nsIPrincipal* aSubjectPrincipal)
-{
-    MOZ_ASSERT(cx);
-    JS::RootedObject jsObject(cx, aJSObject);
-    // Check if the subject is privileged.
-    if (SubjectIsPrivileged())
-        return NS_OK;
-
-    //-- Access tests failed
-    return NS_ERROR_DOM_XPCONNECT_ACCESS_DENIED;
-}
-
 /////////////////////////////////////////////
 // Method implementing nsIChannelEventSink //
 /////////////////////////////////////////////
 NS_IMETHODIMP
 nsScriptSecurityManager::AsyncOnChannelRedirect(nsIChannel* oldChannel, 
                                                 nsIChannel* newChannel,
                                                 uint32_t redirFlags,
                                                 nsIAsyncVerifyRedirectCallback *cb)