diff options
| author | Allan Sandfeld Jensen <allan.jensen@qt.io> | 2019-02-01 15:57:16 +0100 |
|---|---|---|
| committer | Allan Sandfeld Jensen <allan.jensen@qt.io> | 2019-03-29 09:57:11 +0000 |
| commit | d71b14139612f94a2718a262a29579dc401e6d81 (patch) | |
| tree | a8cbf9c96eae4d733b91bcfbc0254ff8701503d6 | |
| parent | 4d30deb8c6529d2f5e5830997f0ef1744ebc82f4 (diff) | |
| download | qtwebengine-chromium-d71b14139612f94a2718a262a29579dc401e6d81.tar.gz | |
[Backport] Fix for CVE-2019-5759
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 <tkent@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
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 <tkent@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#519}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
Reviewed-by: Michael Brüning <michael.bruning@qt.io>
| -rw-r--r-- | chromium/content/renderer/external_popup_menu_browsertest.cc | 26 | ||||
| -rw-r--r-- | chromium/content/renderer/render_frame_impl.cc | 16 | ||||
| -rw-r--r-- | 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( + "<style>* { margin: 0; } iframe { border: 0; }</style>" + "<body><iframe srcdoc=\"" + "<style>* { margin: 0; }</style><select><option>opt1<option>opt2" + "\"></iframe>" + "<script>" + "onload = function() {" + " const frame = document.querySelector('iframe');" + " frame.contentDocument.querySelector('select').onchange = " + " () => { frame.remove(); };" + "};" + "</script>"); + // 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<blink::WebLocalFrame*>(frame()->GetWebFrame()->FirstChild()); + static_cast<RenderFrameImpl*>(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<ExternalPopupMenu> 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<ExternalPopupMenu> 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); |