Bug 479521: Don't follow unsafe same-site to cross-site redirects. Also fix a bug where reusing a XHR object that had been used cross site could result in the second request being more restrictive than it should be. r/sr=jst

build/automation.py.in

@@ -241,6 +241,7 @@ user_pref("javascript.options.jit.content", true);
 user_pref("gfx.color_management.force_srgb", true);
 user_pref("network.manage-offline-status", false);
 user_pref("security.default_personal_cert", "Select Automatically"); // Need to client auth test be w/o any dialogs
+user_pref("network.http.prompt-temp-redirect", false);
 
 user_pref("camino.warn_when_closing", false); // Camino-only, harmless to others
 """

content/base/src/nsCrossSiteListenerProxy.cpp

@@ -57,6 +57,29 @@
 
 static NS_DEFINE_CID(kCParserCID, NS_PARSER_CID);
 
+class nsChannelCanceller
+{
+public:
+  nsChannelCanceller(nsIChannel* aChannel)
+    : mChannel(aChannel)
+  {
+  }
+  ~nsChannelCanceller()
+  {
+    if (mChannel) {
+      mChannel->Cancel(NS_ERROR_DOM_BAD_URI);
+    }
+  }
+
+  void DontCancel()
+  {
+    mChannel = nsnull;
+  }
+
+private:
+  nsIChannel* mChannel;
+};
+
 NS_IMPL_ISUPPORTS4(nsCrossSiteListenerProxy, nsIStreamListener,
                    nsIRequestObserver, nsIChannelEventSink,
                    nsIInterfaceRequestor)
@@ -327,6 +350,7 @@ nsCrossSiteListenerProxy::OnChannelRedirect(nsIChannel *aOldChannel,
                                             nsIChannel *aNewChannel,
                                             PRUint32    aFlags)
 {
+  nsChannelCanceller canceller(aOldChannel);
   nsresult rv;
   if (!NS_IsInternalSameURIRedirect(aOldChannel, aNewChannel, aFlags)) {
     rv = CheckRequestApproved(aOldChannel, PR_TRUE);
@@ -350,7 +374,12 @@ nsCrossSiteListenerProxy::OnChannelRedirect(nsIChannel *aOldChannel,
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
-  return UpdateChannel(aNewChannel);
+  rv = UpdateChannel(aNewChannel);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  canceller.DontCancel();
+  
+  return NS_OK;
 }
 
 nsresult

content/base/src/nsCrossSiteListenerProxy.h

@@ -35,6 +35,9 @@
  *
  * ***** END LICENSE BLOCK ***** */
 
+#ifndef nsCrossSiteListenerProxy_h__
+#define nsCrossSiteListenerProxy_h__
+
 #include "nsIStreamListener.h"
 #include "nsIInterfaceRequestor.h"
 #include "nsCOMPtr.h"
@@ -89,3 +92,5 @@ private:
   nsCString mPreflightMethod;
   nsTArray<nsCString> mPreflightHeaders;
 };
+
+#endif

content/base/src/nsXMLHttpRequest.cpp

@@ -1780,6 +1780,8 @@ CheckMayLoad(nsIPrincipal* aPrincipal, nsIChannel* aChannel)
 nsresult
 nsXMLHttpRequest::CheckChannelForCrossSiteRequest(nsIChannel* aChannel)
 {
+  nsresult rv;
+
   // First check if this is a same-origin request, or if cross-site requests
   // are enabled.
   if ((mState & XML_HTTP_REQUEST_XSITEENABLED) ||
@@ -1790,6 +1792,35 @@ nsXMLHttpRequest::CheckChannelForCrossSiteRequest(nsIChannel* aChannel)
   // This is a cross-site request
   mState |= XML_HTTP_REQUEST_USE_XSITE_AC;
 
+  // Check if we need to do a preflight request.
+  nsCOMPtr<nsIHttpChannel> httpChannel = do_QueryInterface(aChannel);
+  NS_ENSURE_TRUE(httpChannel, NS_ERROR_DOM_BAD_URI);
+    
+  nsCAutoString method;
+  httpChannel->GetRequestMethod(method);
+  if (!mACUnsafeHeaders.IsEmpty() ||
+      HasListenersFor(NS_LITERAL_STRING(UPLOADPROGRESS_STR)) ||
+      (mUpload && mUpload->HasListeners())) {
+    mState |= XML_HTTP_REQUEST_NEED_AC_PREFLIGHT;
+  }
+  else if (method.LowerCaseEqualsLiteral("post")) {
+    nsCAutoString contentTypeHeader;
+    httpChannel->GetRequestHeader(NS_LITERAL_CSTRING("Content-Type"),
+                                  contentTypeHeader);
+
+    nsCAutoString contentType, charset;
+    rv = NS_ParseContentType(contentTypeHeader, contentType, charset);
+    NS_ENSURE_SUCCESS(rv, rv);
+
+    if (!contentType.LowerCaseEqualsLiteral("text/plain")) {
+      mState |= XML_HTTP_REQUEST_NEED_AC_PREFLIGHT;
+    }
+  }
+  else if (!method.LowerCaseEqualsLiteral("get") &&
+           !method.LowerCaseEqualsLiteral("head")) {
+    mState |= XML_HTTP_REQUEST_NEED_AC_PREFLIGHT;
+  }
+
   return NS_OK;
 }
 
@@ -1921,6 +1952,9 @@ nsXMLHttpRequest::OpenRequest(const nsACString& method,
     mState |= XML_HTTP_REQUEST_XSITEENABLED;
   }
 
+  mState &= ~(XML_HTTP_REQUEST_USE_XSITE_AC |
+              XML_HTTP_REQUEST_NEED_AC_PREFLIGHT);
+
   nsCOMPtr<nsIHttpChannel> httpChannel(do_QueryInterface(mChannel));
   if (httpChannel) {
     rv = httpChannel->SetRequestMethod(method);
@@ -2658,35 +2692,6 @@ nsXMLHttpRequest::Send(nsIVariant *aBody)
 
   PRBool withCredentials = !!(mState & XML_HTTP_REQUEST_AC_WITH_CREDENTIALS);
 
-  if (mState & XML_HTTP_REQUEST_USE_XSITE_AC) {
-    // Check if we need to do a preflight request.
-    NS_ENSURE_TRUE(httpChannel, NS_ERROR_DOM_BAD_URI);
-    
-    nsCAutoString method;
-    httpChannel->GetRequestMethod(method);
-    if (!mACUnsafeHeaders.IsEmpty() ||
-        HasListenersFor(NS_LITERAL_STRING(UPLOADPROGRESS_STR)) ||
-        (mUpload && mUpload->HasListeners())) {
-      mState |= XML_HTTP_REQUEST_NEED_AC_PREFLIGHT;
-    }
-    else if (method.LowerCaseEqualsLiteral("post")) {
-      nsCAutoString contentTypeHeader;
-      httpChannel->GetRequestHeader(NS_LITERAL_CSTRING("Content-Type"),
-                                    contentTypeHeader);
-
-      nsCAutoString contentType, charset;
-      NS_ParseContentType(contentTypeHeader, contentType, charset);
-
-      NS_ENSURE_SUCCESS(rv, rv);
-      if (!contentType.LowerCaseEqualsLiteral("text/plain")) {
-        mState |= XML_HTTP_REQUEST_NEED_AC_PREFLIGHT;
-      }
-    }
-    else if (!method.LowerCaseEqualsLiteral("get") &&
-             !method.LowerCaseEqualsLiteral("head")) {
-      mState |= XML_HTTP_REQUEST_NEED_AC_PREFLIGHT;
-    }
-
   // If so, set up the preflight
   if (mState & XML_HTTP_REQUEST_NEED_AC_PREFLIGHT) {
     // Check to see if this initial OPTIONS request has already been cached
@@ -2721,7 +2726,6 @@ nsXMLHttpRequest::Send(nsIVariant *aBody)
       NS_ENSURE_SUCCESS(rv, rv);
     }
   }
-  }
 
   // Hook us up to listen to redirects and the like
   mChannel->GetNotificationCallbacks(getter_AddRefs(mNotificationCallbacks));

content/base/test/file_CrossSiteXHR_server.sjs

@@ -98,6 +98,7 @@ function handleRequest(request, response)
      query.hop = parseInt(query.hop, 10);
      hops = eval(query.hops);
      query.allowOrigin = hops[query.hop-1].allowOrigin;
+     query.allowHeaders = hops[query.hop-1].allowHeaders;
   }
   
   if (query.allowOrigin && (!isPreflight || !query.noAllowPreflight))
@@ -121,7 +122,7 @@ function handleRequest(request, response)
     newURL = hops[query.hop].server +
              "/tests/content/base/test/file_CrossSiteXHR_server.sjs?" +
              "hop=" + (query.hop + 1) + "&hops=" + query.hops;
-    response.setStatusLine(null, 302, "redirect");
+    response.setStatusLine(null, 307, "redirect");
     response.setHeader("Location", newURL);
 
     return;

content/base/test/test_CrossSiteXHR.html

@@ -17,6 +17,11 @@
 <pre id="test">
 <script class="testbody" type="application/javascript;version=1.8">
 
+const runOriginTests = 1;
+const runPreflightTests = 1;
+const runCookieTests = 1;
+const runRedirectTests = 1;
+
 SimpleTest.waitForExplicitFinish();
 
 var origins =
@@ -64,6 +69,11 @@ function runTest() {
   // Test preflight-less requests
   basePath = "/tests/content/base/test/file_CrossSiteXHR_server.sjs?"
   baseURL = "http://localhost:8888" + basePath;
+
+  if (!runOriginTests) {
+    origins = [];
+  }
+
   for each(originEntry in origins) {
     origin = originEntry.origin || originEntry.server;
 
@@ -429,6 +439,10 @@ function runTest() {
                },
                ];
 
+  if (!runPreflightTests) {
+    passTests = failTests = [];
+  }
+
   for each(test in passTests) {
     req = {
       url: baseURL + "&allowOrigin=" + escape(origin) +
@@ -456,6 +470,9 @@ function runTest() {
       req.url += "&allowHeaders=" + escape(test.allowHeaders);
     if ("allowMethods" in test)
       req.url += "&allowMethods=" + escape(test.allowMethods);
+    if (test.body)
+      req.url += "&body=" + escape(test.body);
+
 
     loaderWindow.postMessage(req.toSource(), origin);
 
@@ -492,10 +509,6 @@ function runTest() {
       body: test.body,
     };
 
-    if (test.body) {
-      req.url += "&body=" + escape(test.body);
-    }
-
     if (test.noAllowPreflight)
       req.url += "&noAllowPreflight";
 
@@ -595,6 +608,10 @@ function runTest() {
            },
            ];
 
+  if (!runCookieTests) {
+    tests = [];
+  }
+
   for each(test in tests) {
     req = {
       url: baseURL + "allowOrigin=" + escape(test.origin || origin),
@@ -766,15 +783,125 @@ function runTest() {
                     },
                     ],
            },
+           { pass: 1,
+             method: "POST",
+             body: "hi there",
+             headers: { "Content-Type": "text/plain" },
+             hops: [{ server: "http://example.org",
+                    },
+                    { server: "http://example.com",
+                      allowOrigin: origin,
+                    },
+                    ],
+           },
+           { pass: 0,
+             method: "POST",
+             body: "hi there",
+             headers: { "Content-Type": "text/plain",
+                        "my-header": "myValue",
+                      },
+             hops: [{ server: "http://example.org",
+                    },
+                    { server: "http://example.com",
+                      allowOrigin: origin,
+                      allowHeaders: "my-header",
+                    },
+                    ],
+           },
+           { pass: 0,
+             method: "DELETE",
+             hops: [{ server: "http://example.org",
+                    },
+                    { server: "http://example.com",
+                      allowOrigin: origin,
+                    },
+                    ],
+           },
+           { pass: 0,
+             method: "POST",
+             body: "hi there",
+             headers: { "Content-Type": "text/plain",
+                        "my-header": "myValue",
+                      },
+             hops: [{ server: "http://example.com",
+                      allowOrigin: origin,
+                    },
+                    { server: "http://sub1.test1.example.org",
+                      allowOrigin: origin,
+                    },
+                    ],
+           },
+           { pass: 0,
+             method: "DELETE",
+             hops: [{ server: "http://example.com",
+                      allowOrigin: origin,
+                    },
+                    { server: "http://sub1.test1.example.org",
+                      allowOrigin: origin,
+                    },
+                    ],
+           },
+           { pass: 0,
+             method: "POST",
+             body: "hi there",
+             headers: { "Content-Type": "text/plain",
+                        "my-header": "myValue",
+                      },
+             hops: [{ server: "http://example.com",
+                    },
+                    { server: "http://sub1.test1.example.org",
+                      allowOrigin: origin,
+                      allowHeaders: "my-header",
+                    },
+                    ],
+           },
+           { pass: 1,
+             method: "POST",
+             body: "hi there",
+             headers: { "Content-Type": "text/plain" },
+             hops: [{ server: "http://example.org",
+                    },
+                    { server: "http://example.com",
+                      allowOrigin: origin,
+                    },
+                    ],
+           },
+           { pass: 0,
+             method: "POST",
+             body: "hi there",
+             headers: { "Content-Type": "text/plain",
+                        "my-header": "myValue",
+                      },
+             hops: [{ server: "http://example.com",
+                      allowOrigin: origin,
+                      allowHeaders: "my-header",
+                    },
+                    { server: "http://example.org",
+                      allowOrigin: origin,
+                      allowHeaders: "my-header",
+                    },
+                    ],
+           },
            ];
 
+  if (!runRedirectTests) {
+    tests = [];
+  }
+
   for each(test in tests) {
     req = {
       url: test.hops[0].server + basePath + "hop=1&hops=" +
            escape(test.hops.toSource()),
       method: test.method,
+      headers: test.headers,
+      body: test.body,
     };
 
+    if (test.pass) {
+      if (test.body)
+        req.url += "&body=" + escape(test.body);
+    }
+
     loaderWindow.postMessage(req.toSource(), origin);
 
     res = eval(yield);
@@ -802,7 +929,7 @@ function runTest() {
          "opening,rs1,sending,rs1,loadstart,rs2,rs4,error",
          "wrong events in test for " + test.toSource());
       is(res.progressEvents, 0,
-         "wrong events in test for " + test.toSource());
+         "wrong progressevents in test for " + test.toSource());
     }
   }
 

modules/libpref/src/init/all.js

@@ -656,6 +656,9 @@ pref("network.http.proxy.pipelining", false);
 // Max number of requests in the pipeline
 pref("network.http.pipelining.maxrequests" , 4);
 
+// Prompt for 307 redirects
+pref("network.http.prompt-temp-redirect", true);
+
 // </http>
 
 // If false, remote JAR files that are served with a content type other than

netwerk/protocol/http/src/nsHttpChannel.cpp

@@ -1061,6 +1061,9 @@ nsHttpChannel::ProcessNormal()
 nsresult
 nsHttpChannel::PromptTempRedirect()
 {
+    if (!gHttpHandler->PromptTempRedirect()) {
+        return NS_OK;
+    }
     nsresult rv;
     nsCOMPtr<nsIStringBundleService> bundleService =
             do_GetService(NS_STRINGBUNDLE_CONTRACTID, &rv);

netwerk/protocol/http/src/nsHttpHandler.cpp

@@ -174,6 +174,7 @@ nsHttpHandler::nsHttpHandler()
     , mProduct("Gecko")
     , mUserAgentIsDirty(PR_TRUE)
     , mUseCache(PR_TRUE)
+    , mPromptTempRedirect(PR_TRUE)
     , mSendSecureXSiteReferrer(PR_TRUE)
     , mEnablePersistentHttpsCaching(PR_FALSE)
 {
@@ -1090,6 +1091,13 @@ nsHttpHandler::PrefsChanged(nsIPrefBranch *prefs, const char *pref)
         }
     }
 
+    if (PREF_CHANGED(HTTP_PREF("prompt-temp-redirect"))) {
+        rv = prefs->GetBoolPref(HTTP_PREF("prompt-temp-redirect"), &cVar);
+        if (NS_SUCCEEDED(rv)) {
+            mPromptTempRedirect = cVar;
+        }
+    }
+
     // enable Persistent caching for HTTPS - bug#205921    
     if (PREF_CHANGED(BROWSER_PREF("disk_cache_ssl"))) {
         cVar = PR_FALSE;

netwerk/protocol/http/src/nsHttpHandler.h

@@ -107,6 +107,8 @@ public:
     
     PRBool         CanCacheAllSSLContent()   { return mEnablePersistentHttpsCaching; }
 
+    PRBool         PromptTempRedirect()      { return mPromptTempRedirect; }
+
     nsHttpAuthCache     *AuthCache() { return &mAuthCache; }
     nsHttpConnectionMgr *ConnMgr()   { return mConnMgr; }
 
@@ -299,6 +301,8 @@ private:
     PRPackedBool   mUserAgentIsDirty; // true if mUserAgent should be rebuilt
 
     PRPackedBool   mUseCache;
+
+    PRPackedBool   mPromptTempRedirect;
     // mSendSecureXSiteReferrer: default is false, 
     // if true allow referrer headers between secure non-matching hosts
     PRPackedBool   mSendSecureXSiteReferrer;