PlzNavigate: fix SecurityExploitBrowserTest.MismatchedOriginOnCommit

content/browser/frame_host/render_frame_host_impl.cc

 // Copyright 2013 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.
 
 #include "content/browser/frame_host/render_frame_host_impl.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/command_line.h"
@@ skipping 904 matching lines @@
   RenderProcessHost* process = GetProcess();
 
   // Read the parameters out of the IPC message directly to avoid making another
   // copy when we filter the URLs.
   base::PickleIterator iter(msg);
   FrameHostMsg_DidCommitProvisionalLoad_Params validated_params;
   if (!IPC::ParamTraits<FrameHostMsg_DidCommitProvisionalLoad_Params>::
       Read(&msg, &iter, &validated_params)) {
     bad_message::ReceivedBadMessage(
         process, bad_message::RFH_COMMIT_DESERIALIZATION_FAILED);
+    // PlzNavigate: release the stream now that the renderer is going away.
+    stream_handle_.reset();
     return;
   }
   TRACE_EVENT1("navigation", "RenderFrameHostImpl::OnDidCommitProvisionalLoad",
                "url", validated_params.url.possibly_invalid_spec());
 
   // Sanity-check the page transition for frame type.
   DCHECK_EQ(ui::PageTransitionIsMainFrame(validated_params.transition),
             !GetParent());
 
   // If we're waiting for a cross-site beforeunload ack from this renderer and
@@ skipping 34 matching lines @@
 
   // Attempts to commit certain off-limits URL should be caught more strictly
   // than our FilterURL checks below.  If a renderer violates this policy, it
   // should be killed.
   if (!CanCommitURL(validated_params.url)) {
     VLOG(1) << "Blocked URL " << validated_params.url.spec();
     validated_params.url = GURL(url::kAboutBlankURL);
     // Kills the process.
     bad_message::ReceivedBadMessage(process,
                                     bad_message::RFH_CAN_COMMIT_URL_BLOCKED);
+    // PlzNavigate: release the stream now that the renderer is going away.
+    stream_handle_.reset();
     return;
   }
 
   // Verify that the origin passed from the renderer process is valid and can
   // be allowed to commit in this RenderFrameHost.
   if (!CanCommitOrigin(validated_params.origin, validated_params.url)) {
     bad_message::ReceivedBadMessage(GetProcess(),
                                     bad_message::RFH_INVALID_ORIGIN_ON_COMMIT);
+    // PlzNavigate: release the stream now that the renderer is going away.
+    stream_handle_.reset();
     return;
   }
 
   // Without this check, an evil renderer can trick the browser into creating
   // a navigation entry for a banned URL.  If the user clicks the back button
   // followed by the forward button (or clicks reload, or round-trips through
   // session restore, etc), we'll think that the browser commanded the
   // renderer to load the URL and grant the renderer the privileges to request
   // the URL.  To prevent this attack, we block the renderer from inserting
   // banned URLs into the navigation controller in the first place.
   process->FilterURL(false, &validated_params.url);
   process->FilterURL(true, &validated_params.referrer.url);
   for (std::vector<GURL>::iterator it(validated_params.redirects.begin());
       it != validated_params.redirects.end(); ++it) {
     process->FilterURL(false, &(*it));
   }
   process->FilterURL(true, &validated_params.searchable_form_url);
 
   // Without this check, the renderer can trick the browser into using
   // filenames it can't access in a future session restore.
   if (!render_view_host_->CanAccessFilesOfPageState(
           validated_params.page_state)) {
     bad_message::ReceivedBadMessage(
         GetProcess(), bad_message::RFH_CAN_ACCESS_FILES_OF_PAGE_STATE);
+    // PlzNavigate: release the stream now that the renderer is going away.
+    stream_handle_.reset();

[nasko, 2016/03/18 20:41:14]

Instead of putting these in all cases where we kill the renderer, why not
include the code in ::RenderProcessGone? This will be a good catch all and less
prone to people forgetting to add this call.

[clamy, 2016/03/22 16:46:55]

Actually we already do (in the SiteInstance version which calls
ResetLoadingState). Removed those calls.

     return;
   }
 
   // If the URL does not match what the NavigationHandle expects, treat the
   // commit as a new navigation. This can happen if an ongoing slow
   // same-process navigation is interrupted by a synchronous renderer-initiated
   // navigation.
   // TODO(csharrison): Data navigations loaded with LoadDataWithBaseURL get
   // reset here, because the NavigationHandle tracks the URL but the
   // validated_params.url tracks the data. The trick of saving the old entry ids
@@ skipping 1676 matching lines @@
   FrameTreeNode* focused_frame_tree_node = frame_tree_->GetFocusedFrame();
   if (!focused_frame_tree_node)
     return;
   RenderFrameHostImpl* focused_frame =
       focused_frame_tree_node->current_frame_host();
   DCHECK(focused_frame);
   dst->focused_tree_id = focused_frame->GetAXTreeID();
 }
 
 }  // namespace content