From 2a31e0feae1597c5a3cc3b96860b83bf4f624a35 Mon Sep 17 00:00:00 2001 From: "jst%mozilla.org" Date: Fri, 25 Jan 2008 21:49:11 +0000 Subject: [PATCH] Fixing bug 397791. Prevent document principals from ever changing, and make us not use XOWs for same origin document objects. r=jonas@sickin.cc, sr=bzbarsky@mit.edu --- content/html/document/src/nsHTMLDocument.cpp | 41 +++-- content/xml/document/src/nsXMLDocument.cpp | 176 +++++++------------ content/xml/document/src/nsXMLDocument.h | 7 - js/src/xpconnect/src/XPCWrapper.h | 1 - 4 files changed, 89 insertions(+), 136 deletions(-) diff --git a/content/html/document/src/nsHTMLDocument.cpp b/content/html/document/src/nsHTMLDocument.cpp index 8bc1657b1be..42b578bf718 100644 --- a/content/html/document/src/nsHTMLDocument.cpp +++ b/content/html/document/src/nsHTMLDocument.cpp @@ -2083,17 +2083,39 @@ nsHTMLDocument::OpenCommon(const nsACString& aContentType, PRBool aReplace) } nsCOMPtr callerPrincipal; - nsContentUtils::GetSecurityManager()-> - GetSubjectPrincipal(getter_AddRefs(callerPrincipal)); + nsIScriptSecurityManager *secMan = nsContentUtils::GetSecurityManager(); + + secMan->GetSubjectPrincipal(getter_AddRefs(callerPrincipal)); + + if (!callerPrincipal) { + // If we're called from C++ we can't do a document.open w/o + // changing the principal of the document to something like + // about:blank (as that's the only sane thing to do when we don't + // know the origin of this call), and since we can't change the + // principals of a document for security reasons we'll have to + // refuse to go ahead with this call. + + return NS_ERROR_DOM_SECURITY_ERR; + } + + // We're called from script. Make sure the script is from the same + // origin, not just that the caller can access the document. This is + // needed to keep document principals from ever changing, which is + // needed because of the way we use our XOW code, and is a sane + // thing to do anyways. + + PRBool equals = PR_FALSE; + if (NS_FAILED(callerPrincipal->Equals(NodePrincipal(), &equals)) || + !equals) { + return NS_ERROR_DOM_SECURITY_ERR; + } // The URI for the document after this call. Get it from the calling // principal (if available), or set it to "about:blank" if no // principal is reachable. nsCOMPtr uri; + callerPrincipal->GetURI(getter_AddRefs(uri)); - if (callerPrincipal) { - callerPrincipal->GetURI(getter_AddRefs(uri)); - } if (!uri) { rv = NS_NewURI(getter_AddRefs(uri), NS_LITERAL_CSTRING("about:blank")); @@ -2151,15 +2173,6 @@ nsHTMLDocument::OpenCommon(const nsACString& aContentType, PRBool aReplace) // Remember the old scope in case the call to SetNewDocument changes it. nsCOMPtr oldScope(do_QueryReferent(mScopeObject)); - // If callerPrincipal doesn't match our principal. make sure that - // SetNewDocument gives us a new inner window and clears our scope. - PRBool samePrincipal; - if (!callerPrincipal || - NS_FAILED(callerPrincipal->Equals(NodePrincipal(), &samePrincipal)) || - !samePrincipal) { - SetIsInitialDocument(PR_FALSE); - } - rv = window->SetNewDocument(this, nsnull, PR_FALSE); NS_ENSURE_SUCCESS(rv, rv); diff --git a/content/xml/document/src/nsXMLDocument.cpp b/content/xml/document/src/nsXMLDocument.cpp index f5c263d1ea1..267543188ec 100644 --- a/content/xml/document/src/nsXMLDocument.cpp +++ b/content/xml/document/src/nsXMLDocument.cpp @@ -193,14 +193,8 @@ nsXMLDocument::~nsXMLDocument() mLoopingForSyncLoad = PR_FALSE; } -NS_IMPL_CYCLE_COLLECTION_CLASS(nsXMLDocument) - -NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INHERITED(nsXMLDocument, nsDocument) - NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NSCOMPTR(mScriptContext) -NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END - // QueryInterface implementation for nsXMLDocument -NS_INTERFACE_TABLE_HEAD_CYCLE_COLLECTION_INHERITED(nsXMLDocument) +NS_INTERFACE_TABLE_HEAD(nsXMLDocument) NS_INTERFACE_TABLE_INHERITED3(nsXMLDocument, nsIInterfaceRequestor, nsIChannelEventSink, @@ -227,8 +221,6 @@ void nsXMLDocument::Reset(nsIChannel* aChannel, nsILoadGroup* aLoadGroup) { nsDocument::Reset(aChannel, aLoadGroup); - - mScriptContext = nsnull; } void @@ -240,7 +232,7 @@ nsXMLDocument::ResetToURI(nsIURI *aURI, nsILoadGroup *aLoadGroup, mChannel->Cancel(NS_BINDING_ABORTED); mChannelIsPending = nsnull; } - + nsDocument::ResetToURI(aURI, aLoadGroup, aPrincipal); } @@ -289,27 +281,19 @@ nsXMLDocument::OnChannelRedirect(nsIChannel *aOldChannel, nsIScriptSecurityManager *secMan = nsContentUtils::GetSecurityManager(); - if (mScriptContext && !mCrossSiteAccessEnabled) { - nsCOMPtr stack(do_GetService("@mozilla.org/js/xpc/ContextStack;1", & rv)); - if (NS_FAILED(rv)) - return rv; + nsCOMPtr oldURI; + rv = aOldChannel->GetURI(getter_AddRefs(oldURI)); + NS_ENSURE_SUCCESS(rv, rv); - JSContext *cx = (JSContext *)mScriptContext->GetNativeContext(); - if (!cx) - return NS_ERROR_UNEXPECTED; + nsCOMPtr newURI; + rv = aNewChannel->GetURI(getter_AddRefs(newURI)); + NS_ENSURE_SUCCESS(rv, rv); - stack->Push(cx); + rv = nsContentUtils::GetSecurityManager()-> + CheckSameOriginURI(oldURI, newURI, PR_TRUE); - rv = secMan->CheckSameOrigin(nsnull, newLocation); - - stack->Pop(&cx); - - if (NS_FAILED(rv)) { - // The security manager set a pending exception. Since we're - // running under the event loop, we need to report it. - ::JS_ReportPendingException(cx); - return rv; - } + if (NS_FAILED(rv)) { + return rv; } // XXXbz Shouldn't we look at the owner on the new channel at some point? @@ -360,64 +344,21 @@ nsXMLDocument::SetAsync(PRBool aAsync) return NS_OK; } -nsresult -nsXMLDocument::GetLoadGroup(nsILoadGroup **aLoadGroup) -{ - NS_ENSURE_ARG_POINTER(aLoadGroup); - *aLoadGroup = nsnull; - - if (mScriptContext) { - nsCOMPtr window = - do_QueryInterface(mScriptContext->GetGlobalObject()); - - if (window) { - nsCOMPtr domdoc; - window->GetDocument(getter_AddRefs(domdoc)); - nsCOMPtr doc = do_QueryInterface(domdoc); - if (doc) { - *aLoadGroup = doc->GetDocumentLoadGroup().get(); // already_AddRefed - } - } - } - - return NS_OK; -} - - NS_IMETHODIMP nsXMLDocument::Load(const nsAString& aUrl, PRBool *aReturn) { NS_ENSURE_ARG_POINTER(aReturn); *aReturn = PR_FALSE; - nsIScriptContext *callingContext = nsnull; - - nsCOMPtr stack = - do_GetService("@mozilla.org/js/xpc/ContextStack;1"); - if (stack) { - JSContext *cx; - if (NS_SUCCEEDED(stack->Peek(&cx)) && cx) { - callingContext = nsJSUtils::GetDynamicScriptContext(cx); - } - } + nsCOMPtr callingDoc = + do_QueryInterface(nsContentUtils::GetDocumentFromContext()); nsIURI *baseURI = mDocumentURI; nsCAutoString charset; - if (callingContext) { - nsCOMPtr window = - do_QueryInterface(callingContext->GetGlobalObject()); - - if (window) { - nsCOMPtr dom_doc; - window->GetDocument(getter_AddRefs(dom_doc)); - nsCOMPtr doc(do_QueryInterface(dom_doc)); - - if (doc) { - baseURI = doc->GetBaseURI(); - charset = doc->GetDocumentCharacterSet(); - } - } + if (callingDoc) { + baseURI = callingDoc->GetBaseURI(); + charset = callingDoc->GetDocumentCharacterSet(); } // Create a new URI @@ -427,6 +368,41 @@ nsXMLDocument::Load(const nsAString& aUrl, PRBool *aReturn) return rv; } + nsCOMPtr codebase; + NodePrincipal()->GetURI(getter_AddRefs(codebase)); + + // Get security manager, check to see whether the current document + // is allowed to load this URI. It's important to use the current + // document's principal for this check so that we don't end up in a + // case where code with elevated privileges is calling us and + // changing the principal of this document. + nsIScriptSecurityManager *secMan = nsContentUtils::GetSecurityManager(); + + if (codebase) { + rv = secMan->CheckSameOriginURI(codebase, uri, PR_FALSE); + + if (NS_FAILED(rv)) { + return rv; + } + } else { + // We're called from chrome, check to make sure the URI we're + // about to load is also chrome. + + PRBool isChrome = PR_FALSE; + if (NS_FAILED(uri->SchemeIs("chrome", &isChrome)) || !isChrome) { + return NS_ERROR_DOM_SECURITY_ERR; + } + } + + rv = secMan->CheckConnect(nsnull, uri, "XMLDocument", "load"); + if (NS_FAILED(rv)) { + // We need to return success here so that JS will get a proper + // exception thrown later. Native calls should always result in + // CheckConnect() succeeding, but in case JS calls C++ which calls + // this code the exception might be lost. + return NS_OK; + } + // Partial Reset, need to restore principal for security reasons and // event listener manager so that load listeners etc. will // remain. This should be done before the security check is done to @@ -438,48 +414,20 @@ nsXMLDocument::Load(const nsAString& aUrl, PRBool *aReturn) nsCOMPtr elm(mListenerManager); mListenerManager = nsnull; - ResetToURI(uri, nsnull, principal); - - mListenerManager = elm; - - // Get security manager, check to see if we're allowed to load this URI - nsCOMPtr secMan = - do_GetService(NS_SCRIPTSECURITYMANAGER_CONTRACTID, &rv); - if (NS_FAILED(rv)) { - return rv; - } - - rv = secMan->CheckConnect(nsnull, uri, "XMLDocument", "load"); - if (NS_FAILED(rv)) { - // We need to return success here so that JS will get a proper - // exception thrown later. Native calls should always result in - // CheckConnect() succeeding, but in case JS calls C++ which calls - // this code the exception might be lost. - return NS_OK; - } - - // Store script context, if any, in case we encounter redirect - // (because we need it there) - - mScriptContext = callingContext; - - // Find out if UniversalBrowserRead privileges are enabled - we will - // need this in case of a redirect - PRBool crossSiteAccessEnabled; - rv = secMan->IsCapabilityEnabled("UniversalBrowserRead", - &crossSiteAccessEnabled); - if (NS_FAILED(rv)) { - return rv; - } - - mCrossSiteAccessEnabled = crossSiteAccessEnabled; - - // Create a channel // When we are called from JS we can find the load group for the page, // and add ourselves to it. This way any pending requests // will be automatically aborted if the user leaves the page. + nsCOMPtr loadGroup; - GetLoadGroup(getter_AddRefs(loadGroup)); + if (callingDoc) { + loadGroup = callingDoc->GetDocumentLoadGroup(); + } + + ResetToURI(uri, loadGroup, principal); + + mListenerManager = elm; + + // Create a channel nsCOMPtr channel; // nsIRequest::LOAD_BACKGROUND prevents throbber from becoming active, diff --git a/content/xml/document/src/nsXMLDocument.h b/content/xml/document/src/nsXMLDocument.h index cffd484c07f..0491903abd6 100644 --- a/content/xml/document/src/nsXMLDocument.h +++ b/content/xml/document/src/nsXMLDocument.h @@ -95,14 +95,8 @@ public: virtual nsresult Clone(nsINodeInfo *aNodeInfo, nsINode **aResult) const; - NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED_NO_UNLINK(nsXMLDocument, nsDocument) - void SetLoadedAsData(PRBool aLoadedAsData) { mLoadedAsData = aLoadedAsData; } protected: - virtual nsresult GetLoadGroup(nsILoadGroup **aLoadGroup); - - nsCOMPtr mScriptContext; - // mChannelIsPending indicates whether we're currently asynchronously loading // data from mChannel (via document.load() or normal load). It's set to true // when we first find out about the channel (StartDocumentLoad) and set to @@ -110,7 +104,6 @@ protected: // mChannel is also cancelled. Note that if this member is true, mChannel // cannot be null. PRPackedBool mChannelIsPending; - PRPackedBool mCrossSiteAccessEnabled; PRPackedBool mLoadedAsInteractiveData; PRPackedBool mAsync; PRPackedBool mLoopingForSyncLoad; diff --git a/js/src/xpconnect/src/XPCWrapper.h b/js/src/xpconnect/src/XPCWrapper.h index 42771ac8c3f..9b76bf9b722 100644 --- a/js/src/xpconnect/src/XPCWrapper.h +++ b/js/src/xpconnect/src/XPCWrapper.h @@ -87,7 +87,6 @@ XPC_XOW_ClassNeedsXOW(const char *name) // TODO Make a perfect hash of these and use that? return !strcmp(name, "Window") || !strcmp(name, "Location") || - !strcmp(name, "HTMLDocument") || !strcmp(name, "HTMLIFrameElement") || !strcmp(name, "HTMLFrameElement"); }