From d71b14139612f94a2718a262a29579dc401e6d81 Mon Sep 17 00:00:00 2001 From: Allan Sandfeld Jensen Date: Fri, 1 Feb 2019 15:57:16 +0100 Subject: [Backport] Fix for CVE-2019-5759 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Merge "Fix crashes in RenderFrameImpl::OnSelectPopupMenuItem(s)" to M72 branch ExternalPopupMenu::DidSelectItem(s) can delete the RenderFrameImpl. We need to reset external_popup_menu_ before calling it. Bug: 912211 Change-Id: Ia9a628e144464a2ebb14ab77d3a693fd5cead6fc Reviewed-on: https://chromium-review.googlesource.com/c/1381325 Commit-Queue: Kent Tamura Reviewed-by: Avi Drissman Cr-Original-Commit-Position: refs/heads/master@{#618026}(cherry picked from commit 5405341d5cc268a0b2ff0678bd78ddda0892e7ea) Reviewed-on: https://chromium-review.googlesource.com/c/1390879 Reviewed-by: Kent Tamura Cr-Commit-Position: refs/branch-heads/3626@{#519} Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437} Reviewed-by: Michael Brüning --- .../renderer/external_popup_menu_browsertest.cc | 26 ++++++++++++++++++++++ chromium/content/renderer/render_frame_impl.cc | 16 +++++++++---- chromium/content/renderer/render_frame_impl.h | 1 + 3 files changed, 39 insertions(+), 4 deletions(-) diff --git a/chromium/content/renderer/external_popup_menu_browsertest.cc b/chromium/content/renderer/external_popup_menu_browsertest.cc index d5e16432dc0..0b959a46804 100644 --- a/chromium/content/renderer/external_popup_menu_browsertest.cc +++ b/chromium/content/renderer/external_popup_menu_browsertest.cc @@ -12,6 +12,7 @@ #include "content/renderer/render_view_impl.h" #include "testing/gtest/include/gtest/gtest.h" #include "third_party/WebKit/public/platform/WebSize.h" +#include "third_party/WebKit/public/web/WebLocalFrame.h" #include "third_party/WebKit/public/web/WebView.h" // Tests for the external select popup menu (Mac specific). @@ -153,6 +154,31 @@ TEST_F(ExternalPopupMenuRemoveTest, RemoveOnChange) { EXPECT_FALSE(SimulateElementClick(kSelectID)); } +// crbug.com/912211 +TEST_F(ExternalPopupMenuRemoveTest, RemoveFrameOnChange) { + LoadHTML( + "" + "" + ""); + // Open a popup. + SimulatePointClick(gfx::Point(8, 8)); + // Select something on the sub-frame, it causes the frame to be removed from + // the page. + auto* child_web_frame = + static_cast(frame()->GetWebFrame()->FirstChild()); + static_cast(RenderFrame::FromWebFrame(child_web_frame)) + ->OnSelectPopupMenuItem(1); + // The test passes if the test didn't crash and ASAN didn't complain. +} + class ExternalPopupMenuDisplayNoneTest : public ExternalPopupMenuTest { public: ExternalPopupMenuDisplayNoneTest() {} diff --git a/chromium/content/renderer/render_frame_impl.cc b/chromium/content/renderer/render_frame_impl.cc index 5b17851bc39..d6861794bad 100644 --- a/chromium/content/renderer/render_frame_impl.cc +++ b/chromium/content/renderer/render_frame_impl.cc @@ -5667,8 +5667,12 @@ void RenderFrameImpl::OnFindMatchRects(int current_version) { void RenderFrameImpl::OnSelectPopupMenuItem(int selected_index) { if (external_popup_menu_ == NULL) return; - external_popup_menu_->DidSelectItem(selected_index); - external_popup_menu_.reset(); + // We need to reset |external_popup_menu_| before calling DidSelectItem(), + // which might delete |this|. + // See ExternalPopupMenuRemoveTest.RemoveFrameOnChange + std::unique_ptr popup; + popup.swap(external_popup_menu_); + popup->DidSelectItem(selected_index); } #else void RenderFrameImpl::OnSelectPopupMenuItems( @@ -5681,8 +5685,12 @@ void RenderFrameImpl::OnSelectPopupMenuItems( if (!external_popup_menu_) return; - external_popup_menu_->DidSelectItems(canceled, selected_indices); - external_popup_menu_.reset(); + // We need to reset |external_popup_menu_| before calling DidSelectItems(), + // which might delete |this|. + // See ExternalPopupMenuRemoveTest.RemoveFrameOnChange + std::unique_ptr popup; + popup.swap(external_popup_menu_); + popup->DidSelectItems(canceled, selected_indices); } #endif #endif diff --git a/chromium/content/renderer/render_frame_impl.h b/chromium/content/renderer/render_frame_impl.h index f4dd1be46a2..40db09cb18a 100644 --- a/chromium/content/renderer/render_frame_impl.h +++ b/chromium/content/renderer/render_frame_impl.h @@ -723,6 +723,7 @@ class CONTENT_EXPORT RenderFrameImpl friend class RenderAccessibilityImplTest; friend class TestRenderFrame; FRIEND_TEST_ALL_PREFIXES(ExternalPopupMenuDisplayNoneTest, SelectItem); + FRIEND_TEST_ALL_PREFIXES(ExternalPopupMenuRemoveTest, RemoveFrameOnChange); FRIEND_TEST_ALL_PREFIXES(ExternalPopupMenuRemoveTest, RemoveOnChange); FRIEND_TEST_ALL_PREFIXES(ExternalPopupMenuTest, NormalCase); FRIEND_TEST_ALL_PREFIXES(ExternalPopupMenuTest, ShowPopupThenNavigate); -- cgit v1.2.1