Followup patch to bug 425201. Make sure to throw if xhr.open is called with an illegal uri. Also restore the nsIScriptSecurityManager.CheckConnect API as soap still uses it
authorjonas@sicking.cc
Fri, 18 Apr 2008 10:35:55 -0700
changeset 14480 4bad94dd547fe76c22ddbfa35c6e5aec253ae1d2
parent 14479 30631309886f0cba37319694e35c74dba6873af2
child 14481 d304d30bdab071deef29a4fd28c9e8a003dab7d3
push id1
push userroot
push dateTue, 26 Apr 2011 22:38:44 +0000
treeherdermozilla-beta@bfdb6e623a36 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs425201
milestone1.9pre
Followup patch to bug 425201. Make sure to throw if xhr.open is called with an illegal uri. Also restore the nsIScriptSecurityManager.CheckConnect API as soap still uses it
caps/idl/nsIPrincipal.idl
caps/idl/nsIScriptSecurityManager.idl
caps/include/nsScriptSecurityManager.h
caps/src/nsPrincipal.cpp
caps/src/nsScriptSecurityManager.cpp
content/base/src/nsXMLHttpRequest.cpp
content/base/test/Makefile.in
content/base/test/test_bug425201.html
--- a/caps/idl/nsIPrincipal.idl
+++ b/caps/idl/nsIPrincipal.idl
@@ -46,16 +46,25 @@ struct JSContext;
 struct JSPrincipals;
 %}
 
 interface nsIURI;
 
 [ptr] native JSContext(JSContext);
 [ptr] native JSPrincipals(JSPrincipals);
 
+/**
+ * WARNING!! The JEP needs to call GetOrigin()  to support
+ * JavaScript-to-Java LiveConnect.  So every change to the  nsIPrincipal
+ * interface (big enough to change its IID) also breaks JavaScript-to-Java
+ * LiveConnect on mac.
+ *
+ * If you REALLY have to change this interface, please mark your bug as
+ * blocking bug 293973.
+ */
 [scriptable, uuid(b8268b9a-2403-44ed-81e3-614075c92034)]
 interface nsIPrincipal : nsISerializable
 {
     /**
      * Values of capabilities for each principal. Order is
      * significant: if an operation is performed on a set
      * of capabilities, the minimum is computed.
      */
--- a/caps/idl/nsIScriptSecurityManager.idl
+++ b/caps/idl/nsIScriptSecurityManager.idl
@@ -36,31 +36,47 @@
  * ***** END LICENSE BLOCK ***** */
 
 #include "nsISupports.idl"
 #include "nsIPrincipal.idl"
 #include "nsIXPCSecurityManager.idl"
 interface nsIURI;
 interface nsIChannel;
 
-
-[scriptable, uuid(e74487fd-98ef-48d1-b973-3f2938f04e8e)]
+/**
+ * WARNING!! The JEP needs to call GetSubjectPrincipal()
+ * to support JavaScript-to-Java LiveConnect.  So every change to the
+ * nsIScriptSecurityManager interface (big enough to change its IID) also
+ * breaks JavaScript-to-Java LiveConnect on mac.
+ *
+ * If you REALLY have to change this interface, please mark your bug as
+ * blocking bug 293973.
+ */
+[scriptable, uuid(3fffd8e8-3fea-442e-a0ed-2ba81ae197d5)]
 interface nsIScriptSecurityManager : nsIXPCSecurityManager
 {
     ///////////////// Security Checks //////////////////
     /**
      * Checks whether the running script is allowed to access aProperty.
      */
     [noscript] void checkPropertyAccess(in JSContextPtr aJSContext,
                                         in JSObjectPtr aJSObject,
                                         in string aClassName,
                                         in JSVal aProperty,
                                         in PRUint32 aAction);
 
     /**
+     * Checks whether the running script is allowed to connect to aTargetURI
+     */
+    [noscript] void checkConnect(in JSContextPtr aJSContext,
+                                 in nsIURI aTargetURI,
+                                 in string aClassName,
+                                 in string aProperty);
+
+    /**
      * Check that the script currently running in context "cx" can load "uri".
      *
      * Will return error code NS_ERROR_DOM_BAD_URI if the load request 
      * should be denied.
      *
      * @param cx the JSContext of the script causing the load
      * @param uri the URI that is being loaded
      */
--- a/caps/include/nsScriptSecurityManager.h
+++ b/caps/include/nsScriptSecurityManager.h
@@ -403,17 +403,18 @@ public:
      */
     static PRBool SecurityCompareURIs(nsIURI* aSourceURI, nsIURI* aTargetURI);
 
     static nsresult 
     ReportError(JSContext* cx, const nsAString& messageTag,
                 nsIURI* aSource, nsIURI* aTarget);
     static nsresult
     CheckSameOriginPrincipal(nsIPrincipal* aSubject,
-                             nsIPrincipal* aObject);
+                             nsIPrincipal* aObject,
+                             PRBool aIsCheckConnect);
 
     static PRBool
     GetStrictFileOriginPolicy()
     {
         return sStrictFileOriginPolicy;
     }
 
 private:
@@ -440,25 +441,26 @@ private:
     // when this happens -- this means that there was no JS running.
     nsIPrincipal*
     doGetSubjectPrincipal(nsresult* rv);
     
     nsresult
     CheckPropertyAccessImpl(PRUint32 aAction,
                             nsAXPCNativeCallContext* aCallContext,
                             JSContext* cx, JSObject* aJSObject,
-                            nsISupports* aObj,
+                            nsISupports* aObj, nsIURI* aTargetURI,
                             nsIClassInfo* aClassInfo,
                             const char* aClassName, jsval aProperty,
                             void** aCachedClassPolicy);
 
     nsresult
     CheckSameOriginDOMProp(nsIPrincipal* aSubject, 
                            nsIPrincipal* aObject,
-                           PRUint32 aAction);
+                           PRUint32 aAction,
+                           PRBool aIsCheckConnect);
 
     nsresult
     LookupPolicy(nsIPrincipal* principal,
                  ClassInfoData& aClassData, jsval aProperty,
                  PRUint32 aAction,
                  ClassPolicy** aCachedClassPolicy,
                  SecurityLevel* result);
 
--- a/caps/src/nsPrincipal.cpp
+++ b/caps/src/nsPrincipal.cpp
@@ -286,17 +286,18 @@ nsPrincipal::Equals(nsIPrincipal *aOther
       }
 
       // Fall through to the codebase comparison.
     }
 
     // Codebases are equal if they have the same origin.
     *aResult =
       NS_SUCCEEDED(nsScriptSecurityManager::CheckSameOriginPrincipal(this,
-                                                                     aOther));
+                                                                     aOther,
+                                                                     PR_FALSE));
     return NS_OK;
   }
 
   *aResult = PR_TRUE;
   return NS_OK;
 }
 
 NS_IMETHODIMP
--- a/caps/src/nsScriptSecurityManager.cpp
+++ b/caps/src/nsScriptSecurityManager.cpp
@@ -566,21 +566,49 @@ nsScriptSecurityManager::CheckObjectAcce
 NS_IMETHODIMP
 nsScriptSecurityManager::CheckPropertyAccess(JSContext* cx,
                                              JSObject* aJSObject,
                                              const char* aClassName,
                                              jsval aProperty,
                                              PRUint32 aAction)
 {
     return CheckPropertyAccessImpl(aAction, nsnull, cx, aJSObject,
-                                   nsnull, nsnull,
+                                   nsnull, nsnull, nsnull,
                                    aClassName, aProperty, nsnull);
 }
 
 NS_IMETHODIMP
+nsScriptSecurityManager::CheckConnect(JSContext* cx,
+                                      nsIURI* aTargetURI,
+                                      const char* aClassName,
+                                      const char* aPropertyName)
+{
+    // Get a context if necessary
+    if (!cx)
+    {
+        cx = GetCurrentJSContext();
+        if (!cx)
+            return NS_OK; // No JS context, so allow the load
+    }
+
+    nsresult rv = CheckLoadURIFromScript(cx, aTargetURI);
+    if (NS_FAILED(rv)) return rv;
+
+    JSAutoRequest ar(cx);
+
+    JSString* propertyName = ::JS_InternString(cx, aPropertyName);
+    if (!propertyName)
+        return NS_ERROR_OUT_OF_MEMORY;
+
+    return CheckPropertyAccessImpl(nsIXPCSecurityManager::ACCESS_CALL_METHOD, nsnull,
+                                   cx, nsnull, nsnull, aTargetURI,
+                                   nsnull, aClassName, STRING_TO_JSVAL(propertyName), nsnull);
+}
+
+NS_IMETHODIMP
 nsScriptSecurityManager::CheckSameOrigin(JSContext* cx,
                                          nsIURI* aTargetURI)
 {
     nsresult rv;
 
     // Get a context if necessary
     if (!cx)
     {
@@ -640,17 +668,17 @@ nsScriptSecurityManager::CheckSameOrigin
     }
     return NS_OK;
 }
 
 nsresult
 nsScriptSecurityManager::CheckPropertyAccessImpl(PRUint32 aAction,
                                                  nsAXPCNativeCallContext* aCallContext,
                                                  JSContext* cx, JSObject* aJSObject,
-                                                 nsISupports* aObj,
+                                                 nsISupports* aObj, nsIURI* aTargetURI,
                                                  nsIClassInfo* aClassInfo,
                                                  const char* aClassName, jsval aProperty,
                                                  void** aCachedClassPolicy)
 {
     nsresult rv;
     nsIPrincipal* subjectPrincipal = GetSubjectPrincipal(cx, &rv);
     if (NS_FAILED(rv))
         return rv;
@@ -715,24 +743,32 @@ nsScriptSecurityManager::CheckPropertyAc
                 nsCOMPtr<nsIPrincipal> principalHolder;
                 nsIPrincipal *objectPrincipal;
                 if(aJSObject)
                 {
                     objectPrincipal = doGetObjectPrincipal(aJSObject);
                     if (!objectPrincipal)
                         rv = NS_ERROR_DOM_SECURITY_ERR;
                 }
+                else if(aTargetURI)
+                {
+                    if (NS_FAILED(GetCodebasePrincipal(
+                          aTargetURI, getter_AddRefs(principalHolder))))
+                        return NS_ERROR_FAILURE;
+
+                    objectPrincipal = principalHolder;
+                }
                 else
                 {
-                    NS_ERROR("CheckPropertyAccessImpl called without a target object");
+                    NS_ERROR("CheckPropertyAccessImpl called without a target object or URL");
                     return NS_ERROR_FAILURE;
                 }
                 if(NS_SUCCEEDED(rv))
                     rv = CheckSameOriginDOMProp(subjectPrincipal, objectPrincipal,
-                                                aAction);
+                                                aAction, aTargetURI != nsnull);
                 break;
             }
         default:
 #ifdef DEBUG_CAPS_CheckPropertyAccessImpl
                 printf("ERROR ");
 #endif
             NS_ERROR("Bad Security Level Value");
             return NS_ERROR_FAILURE;
@@ -857,72 +893,99 @@ nsScriptSecurityManager::CheckPropertyAc
     }
 
     return rv;
 }
 
 /* static */
 nsresult
 nsScriptSecurityManager::CheckSameOriginPrincipal(nsIPrincipal* aSubject,
-                                                  nsIPrincipal* aObject)
+                                                  nsIPrincipal* aObject,
+                                                  PRBool aIsCheckConnect)
 {
     /*
     ** Get origin of subject and object and compare.
     */
     if (aSubject == aObject)
         return NS_OK;
 
+    // These booleans are only used when !aIsCheckConnect.  Default
+    // them to false, and change if that turns out wrong.
     PRBool subjectSetDomain = PR_FALSE;
     PRBool objectSetDomain = PR_FALSE;
     
     nsCOMPtr<nsIURI> subjectURI;
     nsCOMPtr<nsIURI> objectURI;
 
-    aSubject->GetDomain(getter_AddRefs(subjectURI));
-    if (!subjectURI) {
+    if (aIsCheckConnect)
+    {
+        // Don't use domain for CheckConnect calls, since that's called for
+        // data-only load checks like XMLHTTPRequest (bug 290100).
         aSubject->GetURI(getter_AddRefs(subjectURI));
-    } else {
-        subjectSetDomain = PR_TRUE;
+        aObject->GetURI(getter_AddRefs(objectURI));
     }
-
-    aObject->GetDomain(getter_AddRefs(objectURI));
-    if (!objectURI) {
-        aObject->GetURI(getter_AddRefs(objectURI));
-    } else {
-        objectSetDomain = PR_TRUE;
+    else
+    {
+        aSubject->GetDomain(getter_AddRefs(subjectURI));
+        if (!subjectURI) {
+            aSubject->GetURI(getter_AddRefs(subjectURI));
+        } else {
+            subjectSetDomain = PR_TRUE;
+        }
+
+        aObject->GetDomain(getter_AddRefs(objectURI));
+        if (!objectURI) {
+            aObject->GetURI(getter_AddRefs(objectURI));
+        } else {
+            objectSetDomain = PR_TRUE;
+        }
     }
 
     if (SecurityCompareURIs(subjectURI, objectURI))
     {   // If either the subject or the object has changed its principal by
         // explicitly setting document.domain then the other must also have
         // done so in order to be considered the same origin. This prevents
         // DNS spoofing based on document.domain (154930)
 
+        // But this restriction does not apply to CheckConnect calls, since
+        // that's called for data-only load checks like XMLHTTPRequest where
+        // we ignore domain (bug 290100).
+        if (aIsCheckConnect)
+            return NS_OK;
+
         // If both or neither explicitly set their domain, allow the access
         if (subjectSetDomain == objectSetDomain)
             return NS_OK;
     }
 
     /*
     ** Access tests failed, so now report error.
     */
     return NS_ERROR_DOM_PROP_ACCESS_DENIED;
 }
 
 
 nsresult
 nsScriptSecurityManager::CheckSameOriginDOMProp(nsIPrincipal* aSubject,
                                                 nsIPrincipal* aObject,
-                                                PRUint32 aAction)
+                                                PRUint32 aAction,
+                                                PRBool aIsCheckConnect)
 {
     nsresult rv;
-    PRBool subsumes;
-    rv = aSubject->Subsumes(aObject, &subsumes);
-    if (NS_SUCCEEDED(rv) && !subsumes) {
-        rv = NS_ERROR_DOM_PROP_ACCESS_DENIED;
+    if (aIsCheckConnect) {
+        // Don't do equality compares, just do a same-origin compare,
+        // since the object principal isn't a real principal, just a
+        // GetCodebasePrincipal() on whatever URI we started with.
+        rv = CheckSameOriginPrincipal(aSubject, aObject, aIsCheckConnect);
+    } else {
+        PRBool subsumes;
+        rv = aSubject->Subsumes(aObject, &subsumes);
+        if (NS_SUCCEEDED(rv) && !subsumes) {
+            rv = NS_ERROR_DOM_PROP_ACCESS_DENIED;
+        }
     }
     
     if (NS_SUCCEEDED(rv))
         return NS_OK;
 
     /*
     * Content can't ever touch chrome (we check for UniversalXPConnect later)
     */
@@ -3057,17 +3120,17 @@ nsScriptSecurityManager::CanAccess(PRUin
                                    JSContext* cx,
                                    JSObject* aJSObject,
                                    nsISupports* aObj,
                                    nsIClassInfo* aClassInfo,
                                    jsval aPropertyName,
                                    void** aPolicy)
 {
     return CheckPropertyAccessImpl(aAction, aCallContext, cx,
-                                   aJSObject, aObj, aClassInfo,
+                                   aJSObject, aObj, nsnull, aClassInfo,
                                    nsnull, aPropertyName, aPolicy);
 }
 
 nsresult
 nsScriptSecurityManager::CheckXPCPermissions(nsISupports* aObj,
                                              const char* aObjectSecurityLevel)
 {
     //-- Check for the all-powerful UniversalXPConnect privilege
--- a/content/base/src/nsXMLHttpRequest.cpp
+++ b/content/base/src/nsXMLHttpRequest.cpp
@@ -1270,17 +1270,17 @@ nsXMLHttpRequest::Open(const nsACString&
     }
 
     // Find out if UniversalBrowserRead privileges are enabled
     if (nsContentUtils::IsCallerTrustedForRead()) {
       mState |= XML_HTTP_REQUEST_XSITEENABLED;
     } else {
       mState &= ~XML_HTTP_REQUEST_XSITEENABLED;
       rv = mPrincipal->CheckMayLoad(targetURI, PR_TRUE);
-      NS_ENSURE_SUCCESS(rv, NS_OK);
+      NS_ENSURE_SUCCESS(rv, rv);
     }
 
     if (argc > 2) {
       JSAutoRequest ar(cx);
       JSBool asyncBool;
       ::JS_ValueToBoolean(cx, argv[2], &asyncBool);
       async = (PRBool)asyncBool;
 
--- a/content/base/test/Makefile.in
+++ b/content/base/test/Makefile.in
@@ -169,16 +169,17 @@ include $(topsrcdir)/config/rules.mk
 		file_XHR_pass2.txt \
 		file_XHR_pass3.txt \
 		file_XHR_pass3.txt^headers^ \
 		file_XHR_fail1.txt \
 		file_XHR_fail1.txt^headers^ \
 		test_bug428847.html \
 		file_bug428847-1.xhtml \
 		file_bug428847-2.xhtml \
+		test_bug425201.html \
 	$(NULL)
 
 libs:: $(_TEST_FILES)
 	$(INSTALL) $(foreach f,$^,"$f") $(DEPTH)/_tests/testing/mochitest/tests/$(relativesrcdir)
 
 check::
 	@$(EXIT_ON_ERROR) \
 	for f in $(subst .cpp,,$(CPP_UNIT_TESTS)); do \
new file mode 100644
--- /dev/null
+++ b/content/base/test/test_bug425201.html
@@ -0,0 +1,48 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=425201
+-->
+<head>
+  <title>Test for Bug 425201</title>
+  <script type="text/javascript" src="/MochiKit/MochiKit.js"></script>
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=425201">Mozilla Bug 425201</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+  
+</div>
+<pre id="test">
+<script class="testbody" type="text/javascript">
+
+/** Test for Bug 425201 **/
+
+var req = new XMLHttpRequest();
+
+var threw = false;
+try {
+  req.open("GET", "http://www.example.com/", false);
+}
+catch (e) {
+  threw = true;
+}
+
+ok(threw, "Open should have thrown");
+
+threw = false;
+try {
+  req.send(null);
+}
+catch (e) {
+  threw = true;
+}
+
+ok(threw, "Send should have thrown");
+
+</script>
+</pre>
+</body>
+</html>