Bug 1031658 - [CSP] Fix userpass removal in PermitsAncestry() and properly handle PermitsAncestry() failures. (r=bz)
authorSid Stamm <sstamm@mozilla.com>
Wed, 02 Jul 2014 14:39:38 -0700
changeset 191948 edcc53a65a0bdac7363f0a0b2f03844f1f887b86
parent 191947 fb9619b783fca1e7ac0f80fef9320868dd4b3778
child 191949 312fd139c63eff84a31e7efb2bb1f0c7a3a34647
push id45710
push usersstamm@mozilla.com
push dateWed, 02 Jul 2014 21:40:03 +0000
treeherdermozilla-inbound@edcc53a65a0b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs1031658
milestone33.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1031658 - [CSP] Fix userpass removal in PermitsAncestry() and properly handle PermitsAncestry() failures. (r=bz)
content/base/src/nsCSPContext.cpp
content/base/src/nsDocument.cpp
--- a/content/base/src/nsCSPContext.cpp
+++ b/content/base/src/nsCSPContext.cpp
@@ -966,23 +966,19 @@ nsCSPContext::AsyncReportViolation(nsISu
 
 /**
  * Based on the given docshell, determines if this CSP context allows the
  * ancestry.
  *
  * In order to determine the URI of the parent document (one causing the load
  * of this protected document), this function obtains the docShellTreeItem,
  * then walks up the hierarchy until it finds a privileged (chrome) tree item.
- * Getting the parent's URI looks like this in pseudocode:
+ * Getting the a tree item's URI looks like this in pseudocode:
  *
- * nsIDocShell->QI(nsIInterfaceRequestor)
- *            ->GI(nsIDocShellTreeItem)
- *            ->QI(nsIInterfaceRequestor)
- *            ->GI(nsIWebNavigation)
- *            ->GetCurrentURI();
+ * nsIDocShellTreeItem->GetDocument()->GetDocumentURI();
  *
  * aDocShell is the docShell for the protected document.
  */
 NS_IMETHODIMP
 nsCSPContext::PermitsAncestry(nsIDocShell* aDocShell, bool* outPermitsAncestry)
 {
   nsresult rv;
 
@@ -994,44 +990,44 @@ nsCSPContext::PermitsAncestry(nsIDocShel
   *outPermitsAncestry = true;
 
   // extract the ancestry as an array
   nsCOMArray<nsIURI> ancestorsArray;
 
   nsCOMPtr<nsIInterfaceRequestor> ir(do_QueryInterface(aDocShell));
   nsCOMPtr<nsIDocShellTreeItem> treeItem(do_GetInterface(ir));
   nsCOMPtr<nsIDocShellTreeItem> parentTreeItem;
-  nsCOMPtr<nsIWebNavigation> webNav;
   nsCOMPtr<nsIURI> currentURI;
   nsCOMPtr<nsIURI> uriClone;
 
   // iterate through each docShell parent item
   while (NS_SUCCEEDED(treeItem->GetParent(getter_AddRefs(parentTreeItem))) &&
          parentTreeItem != nullptr) {
-    ir     = do_QueryInterface(parentTreeItem);
-    NS_ASSERTION(ir, "Could not QI docShellTreeItem to nsIInterfaceRequestor");
 
-    webNav = do_GetInterface(ir);
-    NS_ENSURE_TRUE(webNav, NS_ERROR_FAILURE);
+    nsIDocument* doc = parentTreeItem->GetDocument();
+    NS_ASSERTION(doc, "Could not get nsIDocument from nsIDocShellTreeItem in nsCSPContext::PermitsAncestry");
+    NS_ENSURE_TRUE(doc, NS_ERROR_FAILURE);
 
-    rv = webNav->GetCurrentURI(getter_AddRefs(currentURI));
-    NS_ENSURE_SUCCESS(rv, rv);
+    currentURI = doc->GetDocumentURI();
 
     if (currentURI) {
       // stop when reaching chrome
       bool isChrome = false;
       rv = currentURI->SchemeIs("chrome", &isChrome);
       NS_ENSURE_SUCCESS(rv, rv);
       if (isChrome) { break; }
 
       // delete the userpass from the URI.
       rv = currentURI->CloneIgnoringRef(getter_AddRefs(uriClone));
       NS_ENSURE_SUCCESS(rv, rv);
-      rv = uriClone->SetUserPass(EmptyCString());
-      NS_ENSURE_SUCCESS(rv, rv);
+
+      // We don't care if this succeeds, just want to delete a userpass if
+      // there was one.
+      uriClone->SetUserPass(EmptyCString());
+
 #ifdef PR_LOGGING
       {
       nsAutoCString spec;
       uriClone->GetSpec(spec);
       CSPCONTEXTLOG(("nsCSPContext::PermitsAncestry, found ancestor: %s", spec.get()));
       }
 #endif
       ancestorsArray.AppendElement(uriClone);
--- a/content/base/src/nsDocument.cpp
+++ b/content/base/src/nsDocument.cpp
@@ -2821,19 +2821,18 @@ nsDocument::InitCSP(nsIChannel* aChannel
 
   // ----- Enforce frame-ancestor policy on any applied policies
   nsCOMPtr<nsIDocShell> docShell(mDocumentContainer);
   if (docShell) {
     bool safeAncestry = false;
 
     // PermitsAncestry sends violation reports when necessary
     rv = csp->PermitsAncestry(docShell, &safeAncestry);
-    NS_ENSURE_SUCCESS(rv, rv);
-
-    if (!safeAncestry) {
+
+    if (NS_FAILED(rv) || !safeAncestry) {
 #ifdef PR_LOGGING
       PR_LOG(gCspPRLog, PR_LOG_DEBUG,
               ("CSP doesn't like frame's ancestry, not loading."));
 #endif
       // stop!  ERROR page!
       aChannel->Cancel(NS_ERROR_CSP_FRAME_ANCESTOR_VIOLATION);
     }
   }