From 3cbf2199928337e4930456f1d958d81fa6130f9c Mon Sep 17 00:00:00 2001
From: "jonas@sicking.cc" <jonas@sicking.cc>
Date: Fri, 18 Apr 2008 10:35:55 -0700
Subject: [PATCH] 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              |   9 +++
 caps/idl/nsIScriptSecurityManager.idl  |  20 ++++-
 caps/include/nsScriptSecurityManager.h |   8 +-
 caps/src/nsPrincipal.cpp               |   3 +-
 caps/src/nsScriptSecurityManager.cpp   | 105 ++++++++++++++++++++-----
 content/base/src/nsXMLHttpRequest.cpp  |   2 +-
 content/base/test/Makefile.in          |   1 +
 content/base/test/test_bug425201.html  |  48 +++++++++++
 8 files changed, 168 insertions(+), 28 deletions(-)
 create mode 100644 content/base/test/test_bug425201.html

diff --git a/caps/idl/nsIPrincipal.idl b/caps/idl/nsIPrincipal.idl
index 127f8949ca5..7f669db6dfd 100644
--- a/caps/idl/nsIPrincipal.idl
+++ b/caps/idl/nsIPrincipal.idl
@@ -51,6 +51,15 @@ 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
 {
diff --git a/caps/idl/nsIScriptSecurityManager.idl b/caps/idl/nsIScriptSecurityManager.idl
index 6b3ad08de41..0e34346ff98 100644
--- a/caps/idl/nsIScriptSecurityManager.idl
+++ b/caps/idl/nsIScriptSecurityManager.idl
@@ -41,8 +41,16 @@
 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 //////////////////
@@ -55,6 +63,14 @@ interface nsIScriptSecurityManager : nsIXPCSecurityManager
                                         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".
      *
diff --git a/caps/include/nsScriptSecurityManager.h b/caps/include/nsScriptSecurityManager.h
index d32974dcd2d..c1132bc83d7 100644
--- a/caps/include/nsScriptSecurityManager.h
+++ b/caps/include/nsScriptSecurityManager.h
@@ -408,7 +408,8 @@ public:
                 nsIURI* aSource, nsIURI* aTarget);
     static nsresult
     CheckSameOriginPrincipal(nsIPrincipal* aSubject,
-                             nsIPrincipal* aObject);
+                             nsIPrincipal* aObject,
+                             PRBool aIsCheckConnect);
 
     static PRBool
     GetStrictFileOriginPolicy()
@@ -445,7 +446,7 @@ private:
     CheckPropertyAccessImpl(PRUint32 aAction,
                             nsAXPCNativeCallContext* aCallContext,
                             JSContext* cx, JSObject* aJSObject,
-                            nsISupports* aObj,
+                            nsISupports* aObj, nsIURI* aTargetURI,
                             nsIClassInfo* aClassInfo,
                             const char* aClassName, jsval aProperty,
                             void** aCachedClassPolicy);
@@ -453,7 +454,8 @@ private:
     nsresult
     CheckSameOriginDOMProp(nsIPrincipal* aSubject, 
                            nsIPrincipal* aObject,
-                           PRUint32 aAction);
+                           PRUint32 aAction,
+                           PRBool aIsCheckConnect);
 
     nsresult
     LookupPolicy(nsIPrincipal* principal,
diff --git a/caps/src/nsPrincipal.cpp b/caps/src/nsPrincipal.cpp
index c4efe1ee7c3..330821245a6 100755
--- a/caps/src/nsPrincipal.cpp
+++ b/caps/src/nsPrincipal.cpp
@@ -291,7 +291,8 @@ nsPrincipal::Equals(nsIPrincipal *aOther, PRBool *aResult)
     // Codebases are equal if they have the same origin.
     *aResult =
       NS_SUCCEEDED(nsScriptSecurityManager::CheckSameOriginPrincipal(this,
-                                                                     aOther));
+                                                                     aOther,
+                                                                     PR_FALSE));
     return NS_OK;
   }
 
diff --git a/caps/src/nsScriptSecurityManager.cpp b/caps/src/nsScriptSecurityManager.cpp
index 9cee81a332a..7e3c617460f 100644
--- a/caps/src/nsScriptSecurityManager.cpp
+++ b/caps/src/nsScriptSecurityManager.cpp
@@ -571,10 +571,38 @@ nsScriptSecurityManager::CheckPropertyAccess(JSContext* cx,
                                              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)
@@ -645,7 +673,7 @@ 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)
@@ -720,14 +748,22 @@ nsScriptSecurityManager::CheckPropertyAccessImpl(PRUint32 aAction,
                     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:
@@ -862,7 +898,8 @@ nsScriptSecurityManager::CheckPropertyAccessImpl(PRUint32 aAction,
 /* static */
 nsresult
 nsScriptSecurityManager::CheckSameOriginPrincipal(nsIPrincipal* aSubject,
-                                                  nsIPrincipal* aObject)
+                                                  nsIPrincipal* aObject,
+                                                  PRBool aIsCheckConnect)
 {
     /*
     ** Get origin of subject and object and compare.
@@ -870,24 +907,36 @@ nsScriptSecurityManager::CheckSameOriginPrincipal(nsIPrincipal* aSubject,
     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->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))
@@ -896,6 +945,12 @@ nsScriptSecurityManager::CheckSameOriginPrincipal(nsIPrincipal* aSubject,
         // 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;
@@ -911,13 +966,21 @@ nsScriptSecurityManager::CheckSameOriginPrincipal(nsIPrincipal* aSubject,
 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))
@@ -3062,7 +3125,7 @@ nsScriptSecurityManager::CanAccess(PRUint32 aAction,
                                    void** aPolicy)
 {
     return CheckPropertyAccessImpl(aAction, aCallContext, cx,
-                                   aJSObject, aObj, aClassInfo,
+                                   aJSObject, aObj, nsnull, aClassInfo,
                                    nsnull, aPropertyName, aPolicy);
 }
 
diff --git a/content/base/src/nsXMLHttpRequest.cpp b/content/base/src/nsXMLHttpRequest.cpp
index c4c7731b61e..3eced146e9d 100644
--- a/content/base/src/nsXMLHttpRequest.cpp
+++ b/content/base/src/nsXMLHttpRequest.cpp
@@ -1275,7 +1275,7 @@ nsXMLHttpRequest::Open(const nsACString& method, const nsACString& url)
     } 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) {
diff --git a/content/base/test/Makefile.in b/content/base/test/Makefile.in
index e8a61152f3d..f8aac02efdf 100644
--- a/content/base/test/Makefile.in
+++ b/content/base/test/Makefile.in
@@ -174,6 +174,7 @@ _TEST_FILES = 	test_bug5141.html \
 		test_bug428847.html \
 		file_bug428847-1.xhtml \
 		file_bug428847-2.xhtml \
+		test_bug425201.html \
 	$(NULL)
 
 libs:: $(_TEST_FILES)
diff --git a/content/base/test/test_bug425201.html b/content/base/test/test_bug425201.html
new file mode 100644
index 00000000000..a1e2d3ff669
--- /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>