From 98b32df60a138c6d34721a9d605f3c76c440fe9a Mon Sep 17 00:00:00 2001
From: Jack Miller <jackmil@google.com>
Date: Mon, 29 Jun 2026 11:37:32 -0700
Subject: [PATCH] Prevent unintentionally submitting file dialog when Enter is held down

If a user is holding down the Enter key when a website triggers a file
dialog on Windows, keyboard auto-repeat can immediately confirm and
submit the default selection.

This change mitigates this exploit by detecting if the Enter key is
already pressed when the file dialog opens. If so, dialog confirmations
are blocked until the user physically releases the Enter key.

Bug: 517192965
Change-Id: Ia75a56d738e1212ba565d309ec28b24a0c264174
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7928786
Reviewed-by: Foromo Daniel Soromou <koretadaniel@chromium.org>
Reviewed-by: David Bienvenu <davidbienvenu@chromium.org>
Reviewed-by: Robert Liao <robliao@chromium.org>
Commit-Queue: Jack Miller <jackmil@google.com>
Cr-Commit-Position: refs/heads/main@{#1654225}
---
diff --git a/tools/metrics/histograms/metadata/windows/histograms.xml b/tools/metrics/histograms/metadata/windows/histograms.xml
index 8f31ff8..7734fee 100644
--- a/tools/metrics/histograms/metadata/windows/histograms.xml
+++ b/tools/metrics/histograms/metadata/windows/histograms.xml
@@ -509,6 +509,17 @@
   </summary>
 </histogram>
 
+<histogram name="Windows.SetWindowsHookEx.Result" enum="WinGetLastError"
+    expires_after="2027-06-01">
+  <owner>jackmil@google.com</owner>
+  <owner>robliao@chromium.org</owner>
+  <summary>
+    Records the result of calling SetWindowsHookEx inside
+    SafeAcceptFileDialogEventHandler every time a hook registration is
+    attempted.
+  </summary>
+</histogram>
+
 <histogram name="Windows.ShouldPinToTaskbarResult{PinAppToTaskbarChannel}"
     enum="TaskbarPinResult" expires_after="2026-12-16">
   <owner>davidbienvenu@chromium.org</owner>
diff --git a/ui/shell_dialogs/execute_select_file_win.cc b/ui/shell_dialogs/execute_select_file_win.cc
index b9f46bd..4785be9 100644
--- a/ui/shell_dialogs/execute_select_file_win.cc
+++ b/ui/shell_dialogs/execute_select_file_win.cc
@@ -8,6 +8,7 @@
 #include <wrl/client.h>
 
 #include <memory>
+#include <optional>
 
 #include "base/check.h"
 #include "base/feature_list.h"
@@ -23,6 +24,7 @@
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/shell_dialogs/auto_close_dialog_event_handler_win.h"
 #include "ui/shell_dialogs/base_shell_dialog_win.h"
+#include "ui/shell_dialogs/safe_accept_file_dialog_event_handler_win.h"
 #include "ui/shell_dialogs/select_file_utils_win.h"
 #include "ui/strings/grit/ui_strings.h"
 
@@ -33,36 +35,28 @@
 // Stop switch for the AutoCloseDialogEventHandler.
 BASE_FEATURE(kAutoCloseFileDialogs, base::FEATURE_ENABLED_BY_DEFAULT);
 
-// RAII wrapper around AutoCloseDialogEventHandler.
-class ScopedAutoCloseDialogEventHandler {
+// Stop switch for the `SafeAcceptFileDialogEventHandler`.
+BASE_FEATURE(kSafeAcceptFileDialogs, base::FEATURE_ENABLED_BY_DEFAULT);
+
+// RAII wrapper around a single `IFileDialogEvents` registration.
+class ScopedFileDialogEvents {
  public:
-  ScopedAutoCloseDialogEventHandler(HWND owner_window, IFileDialog* file_dialog)
+  ScopedFileDialogEvents(IFileDialog* file_dialog, IFileDialogEvents* handler)
       : file_dialog_(file_dialog) {
     CHECK(file_dialog_);
-
-    if (!owner_window) {
-      return;
-    }
-
-    if (!base::FeatureList::IsEnabled(kAutoCloseFileDialogs)) {
-      return;
-    }
-
-    Microsoft::WRL::ComPtr<IFileDialogEvents> dialog_event_handler =
-        Microsoft::WRL::Make<AutoCloseDialogEventHandler>(owner_window);
-    if (!dialog_event_handler) {
-      return;
-    }
-
-    file_dialog_->Advise(dialog_event_handler.Get(), &cookie_);
+    CHECK(handler);
+    file_dialog_->Advise(handler, &cookie_);
   }
 
-  ~ScopedAutoCloseDialogEventHandler() {
+  ~ScopedFileDialogEvents() {
     if (cookie_) {
       file_dialog_->Unadvise(cookie_);
     }
   }
 
+  ScopedFileDialogEvents(const ScopedFileDialogEvents&) = delete;
+  ScopedFileDialogEvents& operator=(const ScopedFileDialogEvents&) = delete;
+
  private:
   Microsoft::WRL::ComPtr<IFileDialog> file_dialog_;
   DWORD cookie_ = 0;
@@ -196,10 +190,20 @@
 
   file_save_dialog->SetDefaultExtension(def_ext.c_str());
 
-  // This handler auto-closes the file dialog if its owner window is closed.
-  auto auto_close_dialog_event_handler =
-      std::make_unique<ScopedAutoCloseDialogEventHandler>(
-          owner, file_save_dialog.Get());
+  // Set up event handlers for the dialog.
+  std::optional<ScopedFileDialogEvents> auto_close_event;
+  if (owner && base::FeatureList::IsEnabled(kAutoCloseFileDialogs)) {
+    auto_close_event.emplace(
+        file_save_dialog.Get(),
+        Microsoft::WRL::Make<AutoCloseDialogEventHandler>(owner).Get());
+  }
+
+  std::optional<ScopedFileDialogEvents> safe_accept_event;
+  if (base::FeatureList::IsEnabled(kSafeAcceptFileDialogs)) {
+    safe_accept_event.emplace(
+        file_save_dialog.Get(),
+        Microsoft::WRL::Make<SafeAcceptFileDialogEventHandler>().Get());
+  }
 
   // Never consider the current scope as hung. The hang watching deadline (if
   // any) is not valid since the user can take unbounded time to choose the
@@ -209,9 +213,9 @@
   HRESULT hr = file_save_dialog->Show(owner);
   BaseShellDialogImpl::DisableOwner(owner);
 
-  // Remove the event handler regardless of the return value of Show().
-  auto_close_dialog_event_handler = nullptr;
-
+  // Remove the event handlers regardless of the return value of `Show()`.
+  auto_close_event.reset();
+  safe_accept_event.reset();
   if (FAILED(hr))
     return false;
 
@@ -263,10 +267,20 @@
     return false;
   }
 
-  // This handler auto-closes the file dialog if its owner window is closed.
-  auto auto_close_dialog_event_handler =
-      std::make_unique<ScopedAutoCloseDialogEventHandler>(
-          owner, file_open_dialog.Get());
+  // Set up event handlers for the dialog.
+  std::optional<ScopedFileDialogEvents> auto_close_event;
+  if (owner && base::FeatureList::IsEnabled(kAutoCloseFileDialogs)) {
+    auto_close_event.emplace(
+        file_open_dialog.Get(),
+        Microsoft::WRL::Make<AutoCloseDialogEventHandler>(owner).Get());
+  }
+
+  std::optional<ScopedFileDialogEvents> safe_accept_event;
+  if (base::FeatureList::IsEnabled(kSafeAcceptFileDialogs)) {
+    safe_accept_event.emplace(
+        file_open_dialog.Get(),
+        Microsoft::WRL::Make<SafeAcceptFileDialogEventHandler>().Get());
+  }
 
   // Never consider the current scope as hung. The hang watching deadline (if
   // any) is not valid since the user can take unbounded time to choose the
@@ -276,8 +290,9 @@
   HRESULT hr = file_open_dialog->Show(owner);
   BaseShellDialogImpl::DisableOwner(owner);
 
-  // Remove the event handler regardless of the return value of Show().
-  auto_close_dialog_event_handler = nullptr;
+  // Remove the event handlers regardless of the return value of `Show()`.
+  auto_close_event.reset();
+  safe_accept_event.reset();
 
   if (FAILED(hr))
     return false;
diff --git a/ui/shell_dialogs/safe_accept_file_dialog_event_handler_win.cc b/ui/shell_dialogs/safe_accept_file_dialog_event_handler_win.cc
new file mode 100644
index 0000000..b60a070
--- /dev/null
+++ b/ui/shell_dialogs/safe_accept_file_dialog_event_handler_win.cc
@@ -0,0 +1,175 @@
+// Copyright 2026 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "ui/shell_dialogs/safe_accept_file_dialog_event_handler_win.h"
+
+#include <windows.h>
+
+#include "base/check.h"
+#include "base/metrics/histogram_functions.h"
+#include "base/threading/thread_checker.h"
+#include "base/time/time.h"
+
+namespace ui {
+namespace {
+
+// Used by static hook callbacks to access the active handler instance.
+SafeAcceptFileDialogEventHandler*& GetInstance() {
+  static SafeAcceptFileDialogEventHandler* instance = nullptr;
+  return instance;
+}
+
+// Windows API `GetKeyState()` bitmask indicating a virtual key is held down.
+constexpr SHORT kKeyDownBitmask = 0x8000;
+
+}  // namespace
+
+SafeAcceptFileDialogEventHandler::SafeAcceptFileDialogEventHandler() {
+  CHECK(!GetInstance());
+  GetInstance() = this;
+}
+
+SafeAcceptFileDialogEventHandler::~SafeAcceptFileDialogEventHandler() {
+  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+
+  CHECK(GetInstance());
+  GetInstance() = nullptr;
+
+  if (message_hook_) {
+    ::UnhookWindowsHookEx(message_hook_);
+  }
+}
+
+HRESULT SafeAcceptFileDialogEventHandler::OnTypeChange(
+    IFileDialog* file_dialog) {
+  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+
+  // `OnTypeChange` will be called early in the dialog's lifecycle to detect if
+  // the user is holding Enter as the dialog is presented. It will also be
+  // invoked multiple times during the lifecycle of the dialog. Only do the
+  // initialization once.
+  if (initialize_called_) {
+    return S_OK;
+  }
+
+  return Initialize(file_dialog);
+}
+
+HRESULT SafeAcceptFileDialogEventHandler::OnFileOk(IFileDialog*) {
+  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+
+  // If acceptance occurs while Enter is physically up, accept immediately.
+  if (!IsEnterKeyDown()) {
+    OnEnterKeyboardReleased();
+    return S_OK;
+  }
+
+  // Enforce a minimum display duration to prevent accidental immediate
+  // acceptances. `AutofillSuggestionController` uses a 500ms delay to prevent
+  // accidental interactions. That value is being reused here.
+  constexpr base::TimeDelta kMinDisplayDuration = base::Milliseconds(500);
+  if (base::TimeTicks::Now() - shown_time_ < kMinDisplayDuration) {
+    return S_FALSE;
+  }
+
+  // If the user was holding Enter when the dialog opened, enforce that they
+  // must physically release it before any acceptance can succeed.
+  if (initial_enter_hold_) {
+    return S_FALSE;
+  }
+
+  return S_OK;
+}
+
+HRESULT SafeAcceptFileDialogEventHandler::OnFolderChange(IFileDialog*) {
+  return E_NOTIMPL;
+}
+
+HRESULT SafeAcceptFileDialogEventHandler::OnFolderChanging(IFileDialog*,
+                                                           IShellItem*) {
+  return E_NOTIMPL;
+}
+
+HRESULT SafeAcceptFileDialogEventHandler::OnSelectionChange(IFileDialog*) {
+  return E_NOTIMPL;
+}
+
+HRESULT SafeAcceptFileDialogEventHandler::OnShareViolation(
+    IFileDialog*,
+    IShellItem*,
+    FDE_SHAREVIOLATION_RESPONSE*) {
+  return E_NOTIMPL;
+}
+
+HRESULT SafeAcceptFileDialogEventHandler::OnOverwrite(IFileDialog*,
+                                                      IShellItem*,
+                                                      FDE_OVERWRITE_RESPONSE*) {
+  return E_NOTIMPL;
+}
+
+HRESULT SafeAcceptFileDialogEventHandler::Initialize(IFileDialog* file_dialog) {
+  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+
+  CHECK(!initialize_called_);
+  initialize_called_ = true;
+
+  shown_time_ = base::TimeTicks::Now();
+
+  // If the user is already holding Enter when the dialog is initialized,
+  // track it to prevent keyjacking via hardware auto-repeat.
+  if (IsEnterKeyDown()) {
+    initial_enter_hold_ = true;
+    CHECK(!message_hook_);
+    message_hook_ = ::SetWindowsHookEx(WH_GETMESSAGE, &MessageHookCallback,
+                                       nullptr, ::GetCurrentThreadId());
+    base::UmaHistogramSparse("Windows.SetWindowsHookEx.Result",
+                             message_hook_ ? 0 : ::GetLastError());
+  }
+
+  return S_OK;
+}
+
+bool SafeAcceptFileDialogEventHandler::IsEnterKeyDown() const {
+  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+  if (key_state_for_testing_.has_value()) {
+    return key_state_for_testing_.value();
+  }
+  return (::GetKeyState(VK_RETURN) & kKeyDownBitmask) != 0;
+}
+
+void SafeAcceptFileDialogEventHandler::OnEnterKeyboardReleased() {
+  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+
+  // Disarm the initial hold mitigation once Enter is physically released.
+  if (initial_enter_hold_) {
+    initial_enter_hold_ = false;
+    if (message_hook_) {
+      ::UnhookWindowsHookEx(message_hook_);
+      message_hook_ = nullptr;
+    }
+  }
+}
+
+// static
+LRESULT CALLBACK
+SafeAcceptFileDialogEventHandler::MessageHookCallback(int code,
+                                                      WPARAM wParam,
+                                                      LPARAM lParam) {
+  // Only intercept messages actively being removed from the queue
+  // (`PM_REMOVE`), to avoid handling the same message multiple times during
+  // `PM_NOREMOVE` peeking.
+  if (code == HC_ACTION && wParam == PM_REMOVE) {
+    auto* msg = reinterpret_cast<MSG*>(lParam);
+
+    // Veto flag is cleared on physical KeyUp for Enter.
+    if (msg->message == WM_KEYUP && msg->wParam == VK_RETURN) {
+      if (GetInstance()) {
+        GetInstance()->OnEnterKeyboardReleased();
+      }
+    }
+  }
+  return ::CallNextHookEx(nullptr, code, wParam, lParam);
+}
+
+}  // namespace ui
diff --git a/ui/shell_dialogs/safe_accept_file_dialog_event_handler_win.h b/ui/shell_dialogs/safe_accept_file_dialog_event_handler_win.h
new file mode 100644
index 0000000..92978bc
--- /dev/null
+++ b/ui/shell_dialogs/safe_accept_file_dialog_event_handler_win.h
@@ -0,0 +1,95 @@
+// Copyright 2026 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef UI_SHELL_DIALOGS_SAFE_ACCEPT_FILE_DIALOG_EVENT_HANDLER_WIN_H_
+#define UI_SHELL_DIALOGS_SAFE_ACCEPT_FILE_DIALOG_EVENT_HANDLER_WIN_H_
+
+#include <shobjidl_core.h>
+#include <wrl.h>
+#include <wrl/client.h>
+
+#include <optional>
+
+#include "base/threading/thread_checker.h"
+#include "base/time/time.h"
+#include "ui/shell_dialogs/shell_dialogs_export.h"
+
+class SafeAcceptFileDialogEventHandlerTest;
+
+namespace ui {
+
+// This `IFileDialogEvents` implementation ensures that the file dialog it is
+// attached to cannot be accepted immediately (within 500ms of opening) and
+// prevents keyjacking by ignoring Enter key presses if the key was already
+// held down when the dialog opened.
... (file diff truncated)

diff --git a/ui/shell_dialogs/select_file_dialog_win_unittest.cc b/ui/shell_dialogs/select_file_dialog_win_unittest.cc
index 7c8bec2f..3223dcf 100644
--- a/ui/shell_dialogs/select_file_dialog_win_unittest.cc
+++ b/ui/shell_dialogs/select_file_dialog_win_unittest.cc
@@ -23,6 +23,7 @@
 #include "base/win/scoped_com_initializer.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/base/l10n/l10n_util.h"
+#include "ui/shell_dialogs/safe_accept_file_dialog_event_handler_win.h"
 #include "ui/shell_dialogs/select_file_dialog.h"
 #include "ui/shell_dialogs/select_file_policy.h"
 #include "ui/shell_dialogs/selected_file_info.h"
@@ -532,3 +533,85 @@
   EXPECT_TRUE(was_cancelled());
   EXPECT_TRUE(selected_paths().empty());
 }
+
+class SafeAcceptFileDialogEventHandlerTest : public testing::Test {
+ public:
+  SafeAcceptFileDialogEventHandlerTest()
+      : task_environment_(base::test::TaskEnvironment::TimeSource::MOCK_TIME) {}
+
+  void CreateHandler() {
+    handler_ = Microsoft::WRL::Make<ui::SafeAcceptFileDialogEventHandler>();
+  }
+
+  void InitializeHandler(bool enter_down) {
+    handler_->InitializeForTesting(enter_down);
+  }
+
+  void SetKeyState(bool is_down) { handler_->SetKeyStateForTesting(is_down); }
+
+  void ReleaseEnterKey() { handler_->OnEnterKeyboardReleased(); }
+
+  HRESULT CallOnFileOk(IFileDialog* dialog = nullptr) {
+    return handler_->OnFileOk(dialog);
+  }
+
+ protected:
+  base::test::TaskEnvironment task_environment_;
+  Microsoft::WRL::ComPtr<ui::SafeAcceptFileDialogEventHandler> handler_;
+};
+
+// Standalone tests validating the logical UI exploit time-delay and keyjacking
+// policies.
+
+TEST_F(SafeAcceptFileDialogEventHandlerTest, MinimumDisplayDuration) {
+  CreateHandler();
+  InitializeHandler(/*enter_down=*/false);
+
+  // User presses Enter to accept the dialog.
+  SetKeyState(/*is_down=*/true);
+
+  // Immediately after initialization, acceptance is vetoed (must display for at
+  // least 500ms).
+  EXPECT_EQ(CallOnFileOk(), S_FALSE);
+
+  // Advance time by 500ms.
+  task_environment_.FastForwardBy(base::Milliseconds(500));
+
+  // Acceptance now succeeds.
+  EXPECT_EQ(CallOnFileOk(), S_OK);
+}
+
+TEST_F(SafeAcceptFileDialogEventHandlerTest, VetoesHeldEnterKeyjacking) {
+  CreateHandler();
+
+  // Simulate that Enter was actively being held down when the dialog opened.
+  InitializeHandler(/*enter_down=*/true);
+
+  // User is still holding Enter down (e.g. keyboard auto-repeat firing).
+  SetKeyState(/*is_down=*/true);
+
+  // Advance time beyond the minimum display duration (500ms).
+  task_environment_.FastForwardBy(base::Milliseconds(600));
+
+  // Acceptance is still vetoed because the initial Enter press was never
+  // released.
+  EXPECT_EQ(CallOnFileOk(), S_FALSE);
+
+  // Simulate physical release of the Enter key.
+  ReleaseEnterKey();
+
+  // Subsequent Enter key press succeeds.
+  EXPECT_EQ(CallOnFileOk(), S_OK);
+}
+
+TEST_F(SafeAcceptFileDialogEventHandlerTest, MouseClickAcceptance) {
+  CreateHandler();
+  InitializeHandler(/*enter_down=*/false);
+
+  // Enter key is NOT down (e.g. mouse click or automated test bot).
+  SetKeyState(/*is_down=*/false);
+
+  // Mouse click acceptance succeeds immediately (0ms) without waiting for
+  // 500ms.
+  EXPECT_EQ(CallOnFileOk(), S_OK);
+}