Fix HistoryEntry corruption when commit isn't for provisional entry (try #2).

content/renderer/history_controller.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 /*
  * Copyright (C) 2006, 2007, 2008, 2009 Apple Inc. All rights reserved.
  * Copyright (C) 2008 Nokia Corporation and/or its subsidiary(-ies)
  * Copyright (C) 2008, 2009 Torch Mobile Inc. All rights reserved.
  *     (http://www.torchmobile.com/)
  *
@@ skipping 170 matching lines @@
 }
 
 void HistoryController::UpdateForCommit(RenderFrameImpl* frame,
                                         const WebHistoryItem& item,
                                         WebHistoryCommitType commit_type,
                                         bool navigation_within_page) {
   switch (commit_type) {
     case blink::WebBackForwardCommit:
       if (!provisional_entry_)
         return;
-      current_entry_.reset(provisional_entry_.release());
+
+      // If the current entry is null, this must be a main frame commit.
+      DCHECK(current_entry_ || frame->IsMainFrame());
+
+      // Commit the provisional entry, but only if it is a plausible transition.
+      // Do not commit it if the navigation is in a subframe and the provisional
+      // entry's main frame item does not match the current entry's main frame,
+      // which can happen if multiple forward navigations occur.  In that case,
+      // committing the provisional entry would corrupt it, leading to a URL
+      // spoof.  See https://crbug.com/597322.  (Note that the race in this bug
+      // does not affect main frame navigations, only navigations in subframes.)
+      //
+      // Note that we cannot compare the provisional entry against |item|, since
+      // |item| may have redirected to a different URL and ISN.  We also cannot
+      // compare against the main frame's URL, since that may have changed due
+      // to a replaceState.  (Even origin can change on replaceState in certain
+      // modes.)
+      //
+      // It would be safe to additionally check the ISNs of all parent frames
+      // (and not just the root), but that is less critical because it won't
+      // lead to a URL spoof.
+      if (frame->IsMainFrame() ||
+          current_entry_->root().itemSequenceNumber() ==
+              provisional_entry_->root().itemSequenceNumber()) {
+        current_entry_.reset(provisional_entry_.release());
+      }
+
+      // We're guaranteed to have a current entry now.
+      DCHECK(current_entry_);
+
       if (HistoryEntry::HistoryNode* node =
               current_entry_->GetHistoryNodeForFrame(frame)) {
         node->set_item(item);
       }
       break;
     case blink::WebStandardCommit:
       CreateNewBackForwardItem(frame, item, navigation_within_page);
       break;
     case blink::WebInitialCommitInChildFrame:
       UpdateForInitialLoadInChildFrame(frame, item);
@@ skipping 47 matching lines @@
     bool clone_children_of_target) {
   if (!current_entry_) {
     current_entry_.reset(new HistoryEntry(new_item));
   } else {
     current_entry_.reset(current_entry_->CloneAndReplace(
         new_item, clone_children_of_target, target_frame, render_view_));
   }
 }
 
 }  // namespace content