Fix for bug 838692 - Navigating named targets from sandboxed iframes. (r=smaug)
authorBob Owen <bobowencode@gmail.com>
Wed, 24 Apr 2013 18:13:25 +0100
changeset 131630 0200fd49efc368d4930fa891027539a515245e70
parent 131629 469e647ccbf5b04df168bc249742eb2c57e1fe14
child 131631 a5b13623631ca32b8eaf8a011ab244921cfd5f58
push id270
push userpvanderbeken@mozilla.com
push dateThu, 06 Mar 2014 09:24:21 +0000
reviewerssmaug
bugs838692
milestone23.0a1
Fix for bug 838692 - Navigating named targets from sandboxed iframes. (r=smaug)
docshell/base/nsDocShell.cpp
docshell/base/nsDocShell.h
--- a/docshell/base/nsDocShell.cpp
+++ b/docshell/base/nsDocShell.cpp
@@ -3037,24 +3037,28 @@ nsDocShell::FindItemWithName(const PRUni
     NS_ENSURE_ARG_POINTER(_retval);
 
     // If we don't find one, we return NS_OK and a null result
     *_retval = nullptr;
 
     if (!*aName)
         return NS_OK;
 
-    if (!aRequestor)
-    {
-        nsCOMPtr<nsIDocShellTreeItem> foundItem;
+    if (aRequestor) {
+        // If aRequestor is not null we don't need to check special names, so
+        // just hand straight off to the search by actual name function.
+        return DoFindItemWithName(aName, aRequestor, aOriginalRequestor,
+                                  _retval);
+    } else {
 
         // This is the entry point into the target-finding algorithm.  Check
         // for special names.  This should only be done once, hence the check
         // for a null aRequestor.
 
+        nsCOMPtr<nsIDocShellTreeItem> foundItem;
         nsDependentString name(aName);
         if (name.LowerCaseEqualsLiteral("_self")) {
             foundItem = this;
         }
         else if (name.LowerCaseEqualsLiteral("_blank"))
         {
             // Just return null.  Caller must handle creating a new window with
             // a blank name himself.
@@ -3093,26 +3097,29 @@ nsDocShell::FindItemWithName(const PRUni
                 // Note: _content should always exist.  If we don't have one
                 // hanging off the treeowner, just create a named window....
                 // so don't return here, in case we did that and can now find
                 // it.                
                 // XXXbz should we be using |root| instead of creating
                 // a new window?
             }
 #endif
+        } else {
+            // Do the search for item by an actual name.
+            DoFindItemWithName(aName, aRequestor, aOriginalRequestor,
+                               getter_AddRefs(foundItem));
         }
 
         if (foundItem && !CanAccessItem(foundItem, aOriginalRequestor)) {
             foundItem = nullptr;
         }
 
+        // DoFindItemWithName only returns active items and we don't check if
+        // the item is active for the special cases.
         if (foundItem) {
-            // We return foundItem here even if it's not an active
-            // item since all the names we've dealt with so far are
-            // special cases that we won't bother looking for further.
 
             // If our document is sandboxed, we need to do some extra checks.
             uint32_t sandboxFlags = 0;
 
             nsCOMPtr<nsIDocument> doc = do_GetInterface(aOriginalRequestor);
 
             if (doc) {
                 sandboxFlags = doc->GetSandboxFlags();
@@ -3124,27 +3131,24 @@ nsDocShell::FindItemWithName(const PRUni
 
                 // Is the found item not a top level browsing context and not ourself ?
                 nsCOMPtr<nsIDocShellTreeItem> selfAsItem = static_cast<nsIDocShellTreeItem *>(this);
                 if (foundItem != root && foundItem != selfAsItem) {
                     // Are we an ancestor of the foundItem ?
                     bool isAncestor = false;
 
                     nsCOMPtr<nsIDocShellTreeItem> parentAsItem;
-                    GetSameTypeParent(getter_AddRefs(parentAsItem));
-
+                    foundItem->GetSameTypeParent(getter_AddRefs(parentAsItem));
                     while (parentAsItem) {
-                        nsCOMPtr<nsIDocShellTreeItem> tmp;
-                        parentAsItem->GetParent(getter_AddRefs(tmp));
-
-                        if (tmp && tmp == selfAsItem) {
+                        if (parentAsItem == selfAsItem) {
                             isAncestor = true;
                             break;
                         }
-                        parentAsItem = tmp;
+                        nsCOMPtr<nsIDocShellTreeItem> tmp = parentAsItem;
+                        tmp->GetSameTypeParent(getter_AddRefs(parentAsItem));
                     }
 
                     if (!isAncestor) {
                         // No, we are not an ancestor and our document is
                         // sandboxed, we can't allow this.
                         foundItem = nullptr;
                     }
                 } else {
@@ -3162,22 +3166,27 @@ nsDocShell::FindItemWithName(const PRUni
                             break;
                         }
                         tmp->GetParent(getter_AddRefs(tmp));
                     }
                 }
             }
 
             foundItem.swap(*_retval);
-            return NS_OK;
-        }
-    }
-
-    // Keep looking
-        
+        }
+        return NS_OK;
+    }
+}
+
+nsresult
+nsDocShell::DoFindItemWithName(const PRUnichar* aName,
+                               nsISupports* aRequestor,
+                               nsIDocShellTreeItem* aOriginalRequestor,
+                               nsIDocShellTreeItem** _retval)
+{
     // First we check our name.
     if (mName.Equals(aName) && ItemIsActive(this) &&
         CanAccessItem(this, aOriginalRequestor)) {
         NS_ADDREF(*_retval = this);
         return NS_OK;
     }
 
     // This QI may fail, but the places where we want to compare, comparing
--- a/docshell/base/nsDocShell.h
+++ b/docshell/base/nsDocShell.h
@@ -870,16 +870,23 @@ protected:
 
 private:
     nsCOMPtr<nsIAtom> mForcedCharset;
     nsCOMPtr<nsIAtom> mParentCharset;
     nsTObserverArray<nsWeakPtr> mPrivacyObservers;
     int32_t           mParentCharsetSource;
     nsCString         mOriginalUriString;
 
+    // Separate function to do the actual name (i.e. not _top, _self etc.)
+    // searching for FindItemWithName.
+    nsresult DoFindItemWithName(const PRUnichar* aName,
+                                nsISupports* aRequestor,
+                                nsIDocShellTreeItem* aOriginalRequestor,
+                                nsIDocShellTreeItem** _retval);
+
 #ifdef DEBUG
     // We're counting the number of |nsDocShells| to help find leaks
     static unsigned long gNumberOfDocShells;
 #endif /* DEBUG */
 
 public:
     class InterfaceRequestorProxy : public nsIInterfaceRequestor {
     public: