Skip to content

Commit

Permalink
chore: cherry-pick d6946b70b431 from chromium (electron#37848)
Browse files Browse the repository at this point in the history
* chore: [22-x-y] cherry-pick d6946b70b431 from chromium

* chore: update patches

---------

Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>
Co-authored-by: electron-patch-conflict-fixer[bot] <83340002+electron-patch-conflict-fixer[bot]@users.noreply.github.com>
  • Loading branch information
3 people committed Apr 11, 2023
1 parent 3cd2f2e commit c875adf
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 0 deletions.
1 change: 1 addition & 0 deletions patches/chromium/.patches
Original file line number Diff line number Diff line change
Expand Up @@ -146,4 +146,5 @@ cherry-pick-b5c9e5efe5dd.patch
cherry-pick-56bd20b295b4.patch
cherry-pick-1235110fce18.patch
cherry-pick-b041159d06ad.patch
cherry-pick-d6946b70b431.patch
cherry-pick-d9081493c4b2.patch
47 changes: 47 additions & 0 deletions patches/chromium/cherry-pick-d6946b70b431.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Dave Tapuska <dtapuska@chromium.org>
Date: Fri, 24 Mar 2023 19:32:54 +0000
Subject: Move the edit commands to an on stack variable

DevTools uses nested event loops and the usage of the class member can
be problematic for iteration because the nested loop can change the
variable's storage causing a UAF.

(cherry picked from commit d9b34f0f3a2d0dd73648eca3ef940fb66806227b)

Bug: 1420510
Change-Id: Ie08a71b60401fa4322cca0cc31062ba64672126a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4355811
Reviewed-by: David Bokan <bokan@chromium.org>
Commit-Queue: Dave Tapuska <dtapuska@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1120123}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4369603
Cr-Commit-Position: refs/branch-heads/5615@{#809}
Cr-Branched-From: 9c6408ef696e83a9936b82bbead3d41c93c82ee4-refs/heads/main@{#1109224}

diff --git a/third_party/blink/renderer/core/frame/web_frame_widget_impl.cc b/third_party/blink/renderer/core/frame/web_frame_widget_impl.cc
index 2779b0a23477d33e747cb0d97079b463b1060652..b4ca94c7b39a090b7d9700cd86f04a71ebdfcf1f 100644
--- a/third_party/blink/renderer/core/frame/web_frame_widget_impl.cc
+++ b/third_party/blink/renderer/core/frame/web_frame_widget_impl.cc
@@ -3182,11 +3182,18 @@ void WebFrameWidgetImpl::AddEditCommandForNextKeyEvent(const WebString& name,
}

bool WebFrameWidgetImpl::HandleCurrentKeyboardEvent() {
- bool did_execute_command = false;
+ if (edit_commands_.empty()) {
+ return false;
+ }
WebLocalFrame* frame = FocusedWebLocalFrameInWidget();
if (!frame)
frame = local_root_;
- for (const auto& command : edit_commands_) {
+ bool did_execute_command = false;
+ // Executing an edit command can run JS and we can end up reassigning
+ // `edit_commands_` so move it to a stack variable before iterating on it.
+ Vector<mojom::blink::EditCommandPtr> edit_commands =
+ std::move(edit_commands_);
+ for (const auto& command : edit_commands) {
// In gtk and cocoa, it's possible to bind multiple edit commands to one
// key (but it's the exception). Once one edit command is not executed, it
// seems safest to not execute the rest.

0 comments on commit c875adf

Please sign in to comment.