[Backport] Clear stale NavigationParams from HistoryController.

This prevents newly created iframes during a back/forward from
targeting the wrong NavigationEntry.

BUG=623319
TEST=See bug comment 20 for repro steps.
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2144823002
(CVE-2016-5130)

Change-Id: I1fb981b020f4f25d3c1f36d0c3e037c8c0ac8ab3
Reviewed-by: Michael Brüning <michael.bruning@qt.io>

2 files changed, 19 insertions, 5 deletions

diff --git a/chromium/content/renderer/history_controller.cc b/chromium/content/renderer/history_controller.cc
index ba22246329b..f2450d725a3 100644
--- a/chromium/content/renderer/history_controller.cc
+++ b/chromium/content/renderer/history_controller.cc
@@ -174,7 +174,8 @@ void HistoryController::UpdateForCommit(RenderFrameImpl* frame,
         // a different frame.  For main frames, it is not safe to leave the
         // current_entry_ in place, which may have a cross-site page and will be
         // included in the PageState for this commit.  Replace it with a new
-        // HistoryEntry corresponding to the commit.
+        // HistoryEntry corresponding to the commit, and clear any stale
+        // NavigationParams which might point to the wrong entry.
         //
         // This will lack any subframe history items that were in the original
         // provisional entry, but we don't know what those were after discarding
@@ -187,8 +188,10 @@ void HistoryController::UpdateForCommit(RenderFrameImpl* frame,
         // main frame case.  Since this bug is not present in the new
         // FrameNavigationEntry-based navigation path (https://crbug.com/236848)
         // we'll wait for that to fix the subframe case.
-        if (frame->GetRenderView()->GetMainRenderFrame() == frame)
+        if (frame->GetRenderView()->GetMainRenderFrame() == frame) {
           current_entry_.reset(new HistoryEntry(item));
+          navigation_params_.reset();
+        }
 
         return;
       }
@@ -224,6 +227,13 @@ void HistoryController::UpdateForCommit(RenderFrameImpl* frame,
 
       if (HistoryEntry::HistoryNode* node =
               current_entry_->GetHistoryNodeForFrame(frame)) {
+        // Clear the children and any NavigationParams if this commit isn't for
+        // the same item.  Otherwise we might have stale data from a race.
+        if (node->item().itemSequenceNumber() != item.itemSequenceNumber()) {
+          node->RemoveChildren();
+          navigation_params_.reset();
+        }
+
         node->set_item(item);
       }
       break;
diff --git a/chromium/content/renderer/history_controller.h b/chromium/content/renderer/history_controller.h
index 4f90fc04e2e..81cdad08def 100644
--- a/chromium/content/renderer/history_controller.h
+++ b/chromium/content/renderer/history_controller.h
@@ -149,11 +149,15 @@ class CONTENT_EXPORT HistoryController {
   // A HistoryEntry representing the page that is being loaded, or an empty
   // scoped_ptr if no page is being loaded.
   scoped_ptr<HistoryEntry> provisional_entry_;
-  // The NavigationParams corresponding to the last load that was initiated by
-  // |GoToEntry|. This is kept around so that it can be passed into existing
-  // frames modified during a history navigation in GoToEntry(), and can be
+
+  // The NavigationParams corresponding to the last back/forward load that was
+  // initiated by |GoToEntry|. This is kept around so that it can be passed into
+  // existing frames affected by a history navigation in GoToEntry(), and can be
   // passed into frames created after the commit that resulted from the
   // navigation in GetItemForNewChildFrame().
+  //
+  // This is reset in UpdateForCommit if we see a commit from a different
+  // navigation, to avoid using stale parameters.
   scoped_ptr<NavigationParams> navigation_params_;
 
   DISALLOW_COPY_AND_ASSIGN(HistoryController);