[Backport] CVE-2017-5019

Fix UaF in RenderFrameImpl::OnBeforeUnload.

BUG=666714

Review-Url: https://codereview.chromium.org/2514323003
Cr-Commit-Position: refs/heads/master@{#434226}
(cherry picked from commit 0dd441a0007aa46917779e782ee9094f111a02b3)

TBR=creis@chromium.org,thestig@chromium.org
NOTRY=true
NOPRESUBMIT=true

Change-Id: I9a80342c23552be89ba952b5c0d841f4538099bb
Review-Url: https://codereview.chromium.org/2541073002
Cr-Commit-Position: refs/branch-heads/2924@{#187}
Cr-Branched-From: 3a87aecc31cd1ffe751dd72c04e5a96a1fc8108a-refs/heads/master@{#433059}
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>

2 files changed, 64 insertions, 3 deletions

diff --git a/chromium/content/renderer/render_frame_impl.cc b/chromium/content/renderer/render_frame_impl.cc
index dfa128cb154..56e1adc3bf1 100644
--- a/chromium/content/renderer/render_frame_impl.cc
+++ b/chromium/content/renderer/render_frame_impl.cc
@@ -1470,12 +1470,15 @@ void RenderFrameImpl::OnBeforeUnload() {
   // it.
   CHECK(!frame_->parent());
 
+  // Save the routing_id, as the RenderFrameImpl can be deleted in
+  // dispatchBeforeUnloadEvent. See https://crbug.com/666714 for details.
+  int routing_id = routing_id_;
+
   base::TimeTicks before_unload_start_time = base::TimeTicks::Now();
   bool proceed = frame_->dispatchBeforeUnloadEvent();
   base::TimeTicks before_unload_end_time = base::TimeTicks::Now();
-  Send(new FrameHostMsg_BeforeUnload_ACK(routing_id_, proceed,
-                                         before_unload_start_time,
-                                         before_unload_end_time));
+  RenderThread::Get()->Send(new FrameHostMsg_BeforeUnload_ACK(
+      routing_id, proceed, before_unload_start_time, before_unload_end_time));
 }
 
 void RenderFrameImpl::OnSwapOut(
diff --git a/chromium/content/renderer/render_view_browsertest.cc b/chromium/content/renderer/render_view_browsertest.cc
index 050d05fa8c8..5337dae887b 100644
--- a/chromium/content/renderer/render_view_browsertest.cc
+++ b/chromium/content/renderer/render_view_browsertest.cc
@@ -2578,6 +2578,64 @@ TEST_F(RenderViewImplTest, HistoryIsProperlyUpdatedOnNavigation) {
       view()->historyForwardListCount() + 1);
 }
 
+// IPC Listener that runs a callback when a console.log() is executed from
+// javascript.
+class ConsoleCallbackFilter : public IPC::Listener {
+ public:
+  explicit ConsoleCallbackFilter(
+      base::Callback<void(const base::string16&)> callback)
+      : callback_(callback) {}
+
+  bool OnMessageReceived(const IPC::Message& msg) override {
+    bool handled = true;
+    IPC_BEGIN_MESSAGE_MAP(ConsoleCallbackFilter, msg)
+      IPC_MESSAGE_HANDLER(FrameHostMsg_DidAddMessageToConsole,
+                          OnDidAddMessageToConsole)
+      IPC_MESSAGE_UNHANDLED(handled = false)
+    IPC_END_MESSAGE_MAP()
+    return handled;
+  }
+
+  void OnDidAddMessageToConsole(int32_t,
+                                const base::string16& message,
+                                int32_t,
+                                const base::string16&) {
+    callback_.Run(message);
+  }
+
+ private:
+  base::Callback<void(const base::string16&)> callback_;
+};
+
+// Tests that there's no UaF after dispatchBeforeUnloadEvent.
+// See https://crbug.com/666714.
+TEST_F(RenderViewImplTest, DispatchBeforeUnloadCanDetachFrame) {
+  LoadHTML(
+      "<script>window.onbeforeunload = function() { "
+      "window.console.log('OnBeforeUnload called'); }</script>");
+
+  // Creates a callback that swaps the frame when the 'OnBeforeUnload called'
+  // log is printed from the beforeunload handler.
+  std::unique_ptr<ConsoleCallbackFilter> callback_filter(
+      new ConsoleCallbackFilter(base::Bind(
+          [](RenderFrameImpl* frame, const base::string16& msg) {
+            // Makes sure this happens during the beforeunload handler.
+            EXPECT_EQ(base::UTF8ToUTF16("OnBeforeUnload called"), msg);
+
+            // Swaps the main frame.
+            frame->OnMessageReceived(FrameMsg_SwapOut(
+                frame->GetRoutingID(), 1, false, FrameReplicationState()));
+          },
+          base::Unretained(frame()))));
+  render_thread_->sink().AddFilter(callback_filter.get());
+
+  // Simulates a BeforeUnload IPC received from the browser.
+  frame()->OnMessageReceived(
+      FrameMsg_BeforeUnload(frame()->GetRoutingID(), false));
+
+  render_thread_->sink().RemoveFilter(callback_filter.get());
+}
+
 TEST_F(RenderViewImplBlinkSettingsTest, Default) {
   DoSetUp();
   EXPECT_FALSE(settings()->viewportEnabled());