From 8915de2bfc0af4f47cd7d638bbbce9e290ba6388 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Thu, 29 Nov 2012 18:23:55 -0500 Subject: [PATCH] Bug 814141. Cross-site redirects of a CORS request should null out the request origin. r=sicking --- content/base/src/nsCrossSiteListenerProxy.cpp | 44 ++++++++++++++++- content/base/src/nsCrossSiteListenerProxy.h | 4 ++ content/base/test/test_CrossSiteXHR.html | 48 +++++++++++++++++-- 3 files changed, 91 insertions(+), 5 deletions(-) diff --git a/content/base/src/nsCrossSiteListenerProxy.cpp b/content/base/src/nsCrossSiteListenerProxy.cpp index 4dced59a5005..10788b6544d2 100644 --- a/content/base/src/nsCrossSiteListenerProxy.cpp +++ b/content/base/src/nsCrossSiteListenerProxy.cpp @@ -350,6 +350,7 @@ nsCORSListenerProxy::nsCORSListenerProxy(nsIStreamListener* aOuter, bool aWithCredentials) : mOuterListener(aOuter), mRequestingPrincipal(aRequestingPrincipal), + mOriginHeaderPrincipal(aRequestingPrincipal), mWithCredentials(aWithCredentials && !gDisableCORSPrivateData), mRequestApproved(false), mHasBeenCrossSite(false), @@ -364,6 +365,7 @@ nsCORSListenerProxy::nsCORSListenerProxy(nsIStreamListener* aOuter, const nsTArray& aPreflightHeaders) : mOuterListener(aOuter), mRequestingPrincipal(aRequestingPrincipal), + mOriginHeaderPrincipal(aRequestingPrincipal), mWithCredentials(aWithCredentials && !gDisableCORSPrivateData), mRequestApproved(false), mHasBeenCrossSite(false), @@ -387,6 +389,7 @@ nsCORSListenerProxy::Init(nsIChannel* aChannel, bool aAllowDataURI) if (NS_FAILED(rv)) { mOuterListener = nullptr; mRequestingPrincipal = nullptr; + mOriginHeaderPrincipal = nullptr; mOuterNotificationCallbacks = nullptr; } return rv; @@ -404,6 +407,8 @@ nsCORSListenerProxy::OnStartRequest(nsIRequest* aRequest, nsCOMPtr uri; NS_GetFinalChannelURI(channel, getter_AddRefs(uri)); if (uri) { + // OK to use mRequestingPrincipal since preflights never get + // redirected. sPreflightCache->RemoveEntries(uri, mRequestingPrincipal); } } @@ -488,7 +493,7 @@ nsCORSListenerProxy::CheckRequestApproved(nsIRequest* aRequest) if (mWithCredentials || !allowedOriginHeader.EqualsLiteral("*")) { nsAutoCString origin; - rv = nsContentUtils::GetASCIIOrigin(mRequestingPrincipal, origin); + rv = nsContentUtils::GetASCIIOrigin(mOriginHeaderPrincipal, origin); NS_ENSURE_SUCCESS(rv, rv); if (!allowedOriginHeader.Equals(origin)) { @@ -623,12 +628,47 @@ nsCORSListenerProxy::AsyncOnChannelRedirect(nsIChannel *aOldChannel, nsCOMPtr oldURI; NS_GetFinalChannelURI(aOldChannel, getter_AddRefs(oldURI)); if (oldURI) { + // OK to use mRequestingPrincipal since preflights never get + // redirected. sPreflightCache->RemoveEntries(oldURI, mRequestingPrincipal); } } aOldChannel->Cancel(NS_ERROR_DOM_BAD_URI); return NS_ERROR_DOM_BAD_URI; } + + if (mHasBeenCrossSite) { + // Once we've been cross-site, cross-origin redirects reset our source + // origin. + nsCOMPtr oldChannelPrincipal; + nsContentUtils::GetSecurityManager()-> + GetChannelPrincipal(aOldChannel, getter_AddRefs(oldChannelPrincipal)); + nsCOMPtr newChannelPrincipal; + nsContentUtils::GetSecurityManager()-> + GetChannelPrincipal(aNewChannel, getter_AddRefs(newChannelPrincipal)); + if (!oldChannelPrincipal || !newChannelPrincipal) { + rv = NS_ERROR_OUT_OF_MEMORY; + } + + if (NS_SUCCEEDED(rv)) { + bool equal; + rv = oldChannelPrincipal->Equals(newChannelPrincipal, &equal); + if (NS_SUCCEEDED(rv)) { + if (!equal) { + // Spec says to set our source origin to a unique origin. + mOriginHeaderPrincipal = do_CreateInstance("@mozilla.org/nullprincipal;1"); + if (!mOriginHeaderPrincipal) { + rv = NS_ERROR_OUT_OF_MEMORY; + } + } + } + } + + if (NS_FAILED(rv)) { + aOldChannel->Cancel(rv); + return rv; + } + } } // Prepare to receive callback @@ -729,7 +769,7 @@ nsCORSListenerProxy::UpdateChannel(nsIChannel* aChannel, bool aAllowDataURI) // Add the Origin header nsAutoCString origin; - rv = nsContentUtils::GetASCIIOrigin(mRequestingPrincipal, origin); + rv = nsContentUtils::GetASCIIOrigin(mOriginHeaderPrincipal, origin); NS_ENSURE_SUCCESS(rv, rv); nsCOMPtr http = do_QueryInterface(aChannel); diff --git a/content/base/src/nsCrossSiteListenerProxy.h b/content/base/src/nsCrossSiteListenerProxy.h index 40a9c215a6a0..1fc34ef4d3e6 100644 --- a/content/base/src/nsCrossSiteListenerProxy.h +++ b/content/base/src/nsCrossSiteListenerProxy.h @@ -66,7 +66,11 @@ private: nsresult CheckRequestApproved(nsIRequest* aRequest); nsCOMPtr mOuterListener; + // The principal that originally kicked off the request nsCOMPtr mRequestingPrincipal; + // The principal to use for our Origin header ("source origin" in spec terms). + // This can get changed during redirects, unlike mRequestingPrincipal. + nsCOMPtr mOriginHeaderPrincipal; nsCOMPtr mOuterNotificationCallbacks; bool mWithCredentials; bool mRequestApproved; diff --git a/content/base/test/test_CrossSiteXHR.html b/content/base/test/test_CrossSiteXHR.html index d94c6022b9c4..49500dc2e7b6 100644 --- a/content/base/test/test_CrossSiteXHR.html +++ b/content/base/test/test_CrossSiteXHR.html @@ -859,7 +859,7 @@ function runTest() { }, ], }, - { pass: 1, + { pass: 0, method: "GET", hops: [{ server: "http://example.com", allowOrigin: origin @@ -869,6 +869,16 @@ function runTest() { }, ], }, + { pass: 1, + method: "GET", + hops: [{ server: "http://example.com", + allowOrigin: origin + }, + { server: "http://example.org", + allowOrigin: "*" + }, + ], + }, { pass: 0, method: "GET", hops: [{ server: "http://example.com", @@ -902,7 +912,7 @@ function runTest() { }, ], }, - { pass: 1, + { pass: 0, method: "GET", hops: [{ server: "http://example.com", allowOrigin: origin @@ -918,6 +928,38 @@ function runTest() { }, ], }, + { pass: 0, + method: "GET", + hops: [{ server: "http://example.com", + allowOrigin: origin + }, + { server: "http://test2.example.org:8000", + allowOrigin: origin + }, + { server: "http://sub2.xn--lt-uia.example.org", + allowOrigin: "*" + }, + { server: "http://sub1.test1.example.org", + allowOrigin: "*" + }, + ], + }, + { pass: 1, + method: "GET", + hops: [{ server: "http://example.com", + allowOrigin: origin + }, + { server: "http://test2.example.org:8000", + allowOrigin: "*" + }, + { server: "http://sub2.xn--lt-uia.example.org", + allowOrigin: "*" + }, + { server: "http://sub1.test1.example.org", + allowOrigin: "*" + }, + ], + }, { pass: 0, method: "GET", hops: [{ server: "http://example.com", @@ -934,7 +976,7 @@ function runTest() { }, ], }, - { pass: 1, + { pass: 0, method: "GET", hops: [{ server: "http://example.com", allowOrigin: origin