Medium chrome Authorization Error 📄 Reporter bug report 🔧 Commit mapped

Overview

Medium
Severity
—
CVSS
No
Exploited ITW
Fixed
Fix Status
ImpactIncorrect authorization in WebProtect
DescriptionIncorrect authorization in WebProtect
ComponentWebProtect
Sub-componentEnterprise>WebProtect
Bug ClassAuthorization Error
Tracker553139506
Fix commitce89b0166e27 (chromium/src) +219/-7
CISA KEVNot listed
CreditedGoogle
Disclosed2026-10-06

Changed Functions

FunctionChangeNotes
DesktopDataControlsDialogDismissTest
chrome/browser/enterprise/data_controls/desktop_data_controls_dialog_browsertest.cc
modified
IN_PROC_BROWSER_TEST_P
chrome/browser/enterprise/data_controls/desktop_data_controls_dialog_browsertest.cc
modified
DataControlsClipboardUtilsBrowserTest
chrome/browser/enterprise/data_protection/data_protection_clipboard_utils_browsertest.cc
modified
DataControlsClipboardUtilsBrowserTestBase
chrome/browser/enterprise/data_protection/data_protection_clipboard_utils_browsertest.cc
modified
DataControlsClipboardUtilsBrowserTest
chrome/browser/enterprise/data_protection/data_protection_clipboard_utils_browsertest.cc
modified
DataControlsClipboardUtilsBrowserTestBase
chrome/browser/enterprise/data_protection/data_protection_clipboard_utils_browsertest.cc
modified
DataControlsClipboardUtilsDismissTest
chrome/browser/enterprise/data_protection/data_protection_clipboard_utils_browsertest.cc
modified

Files Changed

  • chrome/browser/enterprise/data_controls/desktop_data_controls_dialog_browsertest.cc
  • chrome/browser/enterprise/data_controls/desktop_data_controls_dialog.cc
  • chrome/browser/enterprise/data_controls/desktop_data_controls_dialog_test_helper.cc
  • chrome/browser/enterprise/data_controls/desktop_data_controls_dialog_test_helper.h
  • chrome/browser/enterprise/data_protection/data_protection_clipboard_utils_browsertest.cc
From ce89b0166e2734116737e9fd353f3506c41d1c75 Mon Sep 17 00:00:00 2001
From: Dominique Fauteux-Chapleau <domfc@chromium.org>
Date: Tue, 08 Sep 2026 08:13:16 -0700
Subject: [PATCH] Prevent ESC key from bypassing Data Controls dialogs

In DataControlsDialog, clicking the "Cancel" button is used to bypass
enterprise warnings (while the "OK" button acknowledges/closes blocks).
By default, views::DialogDelegate maps the ESC key accelerator to
CancelDialog(), which unintentionally treated pressing ESC as clicking
the "Cancel" button. This resulted in bypassing enterprise warnings and
allowing sensitive data transfers.

This CL sets `set_esc_should_cancel_dialog_override(false)` on
DataControlsDialogDelegate so that pressing ESC dismisses the dialog
without bypassing warnings or allowing blocked actions.

Fixed: b:553139506
Change-Id: I7de498e0fe65b58c64af9a2f04f017e205ee387d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8360109
Auto-Submit: Dominique Fauteux-Chapleau <domfc@chromium.org>
Reviewed-by: Haihan Chen <haihan@google.com>
Commit-Queue: Haihan Chen <haihan@google.com>
Cr-Commit-Position: refs/heads/main@{#1693798}
---
diff --git a/chrome/browser/enterprise/data_controls/desktop_data_controls_dialog.cc b/chrome/browser/enterprise/data_controls/desktop_data_controls_dialog.cc
index 9a3178c..a84334d 100644
--- a/chrome/browser/enterprise/data_controls/desktop_data_controls_dialog.cc
+++ b/chrome/browser/enterprise/data_controls/desktop_data_controls_dialog.cc
@@ -47,6 +47,12 @@
       : type_(type), dialog_(dialog) {
     set_fixed_width(views::LayoutProvider::Get()->GetDistanceMetric(
         views::DISTANCE_MODAL_DIALOG_PREFERRED_WIDTH));
+    // Unlike most dialogs, "esc" is not equivalent to the "cancel" button as it
+    // bypasses warnings for this dialog, so this override is necessary to have
+    // "esc" behave the same as dismissing the dialog without letting data
+    // through.
+    set_esc_should_cancel_dialog_override(false);
+
     // TODO(crbug.com/351342878): Move shared logic for dialog button styling to
     // `DataControlsDialog`.
     // For warning dialogs, "cancel" means "ignore the warning and bypass" and
diff --git a/chrome/browser/enterprise/data_controls/desktop_data_controls_dialog_test_helper.cc b/chrome/browser/enterprise/data_controls/desktop_data_controls_dialog_test_helper.cc
index 4454c243..dad8966f 100644
--- a/chrome/browser/enterprise/data_controls/desktop_data_controls_dialog_test_helper.cc
+++ b/chrome/browser/enterprise/data_controls/desktop_data_controls_dialog_test_helper.cc
@@ -7,6 +7,10 @@
 #include "base/run_loop.h"
 #include "base/task/single_thread_task_runner.h"
 #include "base/test/gtest_util.h"
+#include "ui/base/accelerators/accelerator.h"
+#include "ui/events/keycodes/keyboard_codes.h"
+#include "ui/views/widget/widget.h"
+#include "ui/views/window/dialog_client_view.h"
 
 namespace data_controls {
 
@@ -34,6 +38,7 @@
   dialog_ = dialog;
   dialog_delegate_ = dialog_delegate;
   ASSERT_EQ(dialog->type(), expected_dialog_type_);
+  ASSERT_FALSE(dialog_delegate_->EscShouldCancelDialog());
 }
 
 void DesktopDataControlsDialogTestHelper::OnWidgetInitialized(
@@ -91,6 +96,23 @@
                                 base::Unretained(dialog_delegate_)));
 }
 
+void DesktopDataControlsDialogTestHelper::DismissWithEsc() {
+  // Some platforms crash if the dialog has been accepted/cancelled before fully
+  // launching modally, so to avoid that issue closing the dialog
+  // is done asynchronously.
+  ASSERT_TRUE(dialog_delegate_);
+  base::SingleThreadTaskRunner::GetCurrentDefault()->PostTask(
+      FROM_HERE, base::BindOnce(
+                     [](views::DialogDelegate* dialog_delegate) {
+                       views::DialogClientView* dialog_client_view =
+                           dialog_delegate->GetDialogClientView();
+                       ASSERT_TRUE(dialog_client_view);
+                       dialog_client_view->AcceleratorPressed(
+                           ui::Accelerator(ui::VKEY_ESCAPE, ui::EF_NONE));
+                     },
+                     base::Unretained(dialog_delegate_)));
+}
+
 void DesktopDataControlsDialogTestHelper::WaitForDialogToInitialize() {
   ASSERT_TRUE(dialog_init_loop_);
   dialog_init_loop_->Run();
diff --git a/chrome/browser/enterprise/data_controls/desktop_data_controls_dialog_test_helper.h b/chrome/browser/enterprise/data_controls/desktop_data_controls_dialog_test_helper.h
index 0ac280e..0075dee 100644
--- a/chrome/browser/enterprise/data_controls/desktop_data_controls_dialog_test_helper.h
+++ b/chrome/browser/enterprise/data_controls/desktop_data_controls_dialog_test_helper.h
@@ -35,6 +35,9 @@
   void BypassWarning();
   void CloseDialogWithoutBypass();
 
+  // Mimics the user closing the dialog using the Esc key.
+  void DismissWithEsc();
+
   // Runs `dialog_init_loop_`.
   void WaitForDialogToInitialize();
 
diff --git a/chrome/browser/enterprise/data_controls/desktop_data_controls_dialog_browsertest.cc b/chrome/browser/enterprise/data_controls/desktop_data_controls_dialog_browsertest.cc
index f719f6f..afe92d89 100644
--- a/chrome/browser/enterprise/data_controls/desktop_data_controls_dialog_browsertest.cc
+++ b/chrome/browser/enterprise/data_controls/desktop_data_controls_dialog_browsertest.cc
@@ -17,8 +17,12 @@
 #include "chrome/test/base/in_process_browser_test.h"
 #include "content/public/test/browser_test.h"
 #include "content/public/test/browser_test_utils.h"
+#include "ui/base/accelerators/accelerator.h"
 #include "ui/base/mojom/dialog_button.mojom.h"
+#include "ui/events/keycodes/keyboard_codes.h"
 #include "ui/views/controls/webview/web_dialog_view.h"
+#include "ui/views/widget/widget.h"
+#include "ui/views/window/dialog_client_view.h"
 #include "ui/web_dialogs/test/test_web_dialog_delegate.h"
 
 namespace data_controls {
@@ -62,6 +66,7 @@
 
     ASSERT_EQ(delegate->GetDefaultDialogButton(),
               static_cast<int>(ui::mojom::DialogButton::kOk));
+    ASSERT_FALSE(delegate->EscShouldCancelDialog());
 
     ASSERT_FALSE(delegates_.contains(dialog));
     ASSERT_FALSE(dialog_close_loops_.contains(dialog));
@@ -254,4 +259,74 @@
   CloseDialogsAndWait();
 }
 
+class DesktopDataControlsDialogDismissTest
+    : public DesktopDataControlsDialogTest,
+      public testing::WithParamInterface<DataControlsDialog::Type> {};
+
+INSTANTIATE_TEST_SUITE_P(
+    All,
+    DesktopDataControlsDialogDismissTest,
+    testing::Values(DataControlsDialog::Type::kClipboardPasteWarn,
+                    DataControlsDialog::Type::kClipboardCopyWarn,
+                    DataControlsDialog::Type::kClipboardPasteBlock,
+                    DataControlsDialog::Type::kClipboardCopyBlock,
+                    DataControlsDialog::Type::kClipboardDragBlock));
+
+IN_PROC_BROWSER_TEST_P(DesktopDataControlsDialogDismissTest, EscDoesNotBypass) {
+  base::test::TestFuture<bool> was_bypassed;
+  DesktopDataControlsDialogFactory::GetInstance()->ShowDialogIfNeeded(
+      browser()->GetTabStripModel()->GetActiveWebContents(), GetParam(),
+      was_bypassed.GetCallback());
+
+  ASSERT_EQ(constructor_called_count_, 1u);
+  auto* dialog = delegates_.begin()->first;
+  auto* delegate = delegates_.begin()->second;
+
+  // Closing the dialog with ESC should never bypass the warning or block.
+  base::SingleThreadTaskRunner::GetCurrentDefault()->PostTask(
+      FROM_HERE, base::BindOnce(
+                     [](views::DialogDelegate* delegate) {
+                       views::DialogClientView* dialog_client_view =
+                           delegate->GetDialogClientView();
+                       ASSERT_TRUE(dialog_client_view);
+                       dialog_client_view->AcceleratorPressed(
+                           ui::Accelerator(ui::VKEY_ESCAPE, ui::EF_NONE));
+                     },
+                     base::Unretained(delegate)));
+
+  dialog_close_loops_[dialog]->Run();
+
+  ASSERT_TRUE(was_bypassed.IsReady());
+  EXPECT_FALSE(was_bypassed.Get());
+}
+
+IN_PROC_BROWSER_TEST_P(DesktopDataControlsDialogDismissTest,
+                       CloseWithDialogButton) {
+  base::test::TestFuture<bool> was_bypassed;
+  DesktopDataControlsDialogFactory::GetInstance()->ShowDialogIfNeeded(
+      browser()->GetTabStripModel()->GetActiveWebContents(), GetParam(),
+      was_bypassed.GetCallback());
+
+  ASSERT_EQ(constructor_called_count_, 1u);
+  auto* dialog = delegates_.begin()->first;
+  auto* delegate = delegates_.begin()->second;
+
+  const bool can_bypass =
+      GetParam() == DataControlsDialog::Type::kClipboardPasteWarn ||
+      GetParam() == DataControlsDialog::Type::kClipboardCopyWarn;
+
+  // For warning dialogs, clicking the "cancel" button bypasses the warning.
+  // For block dialogs, clicking the "OK" button acknowledges the block.
+  base::SingleThreadTaskRunner::GetCurrentDefault()->PostTask(
+      FROM_HERE,
+      base::BindOnce(can_bypass ? &views::DialogDelegate::CancelDialog
+                                : &views::DialogDelegate::AcceptDialog,
+                     base::Unretained(delegate)));
+
+  dialog_close_loops_[dialog]->Run();
+
+  ASSERT_TRUE(was_bypassed.IsReady());
+  EXPECT_EQ(was_bypassed.Get(), can_bypass);
+}
+
 }  // namespace data_controls
diff --git a/chrome/browser/enterprise/data_protection/data_protection_clipboard_utils_browsertest.cc b/chrome/browser/enterprise/data_protection/data_protection_clipboard_utils_browsertest.cc
index b4a22d2..285966de 100644
--- a/chrome/browser/enterprise/data_protection/data_protection_clipboard_utils_browsertest.cc
+++ b/chrome/browser/enterprise/data_protection/data_protection_clipboard_utils_browsertest.cc
@@ -7,6 +7,7 @@
 #include "base/functional/bind.h"
 #include "base/functional/callback_helpers.h"
 #include "base/run_loop.h"
+#include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/test/bind.h"
 #include "base/test/metrics/histogram_tester.h"
@@ -40,6 +41,7 @@
 #include "components/enterprise/connectors/core/reporting_test_utils.h"
 #include "components/enterprise/data_controls/content/browser/last_replaced_clipboard_data.h"
 #include "components/enterprise/data_controls/core/browser/features.h"
+#include "components/enterprise/data_controls/core/browser/rule.h"
 #include "components/enterprise/data_controls/core/browser/test_utils.h"
 #include "components/policy/core/common/cloud/cloud_policy_client.h"
 #include "components/policy/core/common/cloud/cloud_policy_core.h"
@@ -95,11 +97,10 @@
 // Tests for functions and classes declared in data_protection_clipboard_utils.h
 // For browser tests that test data protection integration with Chrome's
 // clipboard logic, see clipboard_browsertests.cc
-class DataControlsClipboardUtilsBrowserTest
-    : public MixinBasedInProcessBrowserTest,
-      public testing::WithParamInterface<std::tuple<bool, bool>> {
+class DataControlsClipboardUtilsBrowserTestBase
+    : public MixinBasedInProcessBrowserTest {
  public:
-  DataControlsClipboardUtilsBrowserTest() {
+  DataControlsClipboardUtilsBrowserTestBase() {
     std::vector<base::test::FeatureRef> enabled_features = {
         data_controls::kDataControlsSearchWith,
         enterprise_connectors::kContentAnalysisClipboardCopy,
@@ -117,7 +118,7 @@
     ui::TestClipboard::CreateForCurrentThread();
   }
 
-  ~DataControlsClipboardUtilsBrowserTest() override {
+  ~DataControlsClipboardUtilsBrowserTestBase() override {
     ui::Clipboard::DestroyClipboardForCurrentThread();
   }
 
@@ -201,9 +202,9 @@
 #endif
   }
 
-  bool machine_scope() const { return std::get<0>(GetParam()); }
+  virtual bool machine_scope() const = 0;
 
-  bool use_workspace_urls() const { return std::get<1>(GetParam()); }
+  virtual bool use_workspace_urls() const = 0;
 
   const char* test_url_0() const {
     return use_workspace_urls() ? kWorkspaceUrlForUser0 : kNonWorkspaceUrl;
@@ -261,6 +262,14 @@
       active_user_test_mixin_;
 };
 
+class DataControlsClipboardUtilsBrowserTest
+    : public DataControlsClipboardUtilsBrowserTestBase,
+      public testing::WithParamInterface<std::tuple<bool, bool>> {
+ public:
+  bool machine_scope() const override { return std::get<0>(GetParam()); }
+  bool use_workspace_urls() const override { return std::get<1>(GetParam()); }
+};
+
 IN_PROC_BROWSER_TEST_P(DataControlsClipboardUtilsBrowserTest,
                        CopyContentAnalysisWarningCanceled) {
   active_user_test_mixin_->SetFakeCookieValue();
@@ -680,6 +689,103 @@
   run_loop.Run();
 }
 
+class DataControlsClipboardUtilsDismissTest
+    : public DataControlsClipboardUtilsBrowserTestBase,
+      public testing::WithParamInterface<
+          std::tuple<bool, bool, data_controls::Rule::Level>> {
+ public:
+  bool machine_scope() const override { return std::get<0>(GetParam()); }
+  bool use_workspace_urls() const override { return std::get<1>(GetParam()); }
+  data_controls::Rule::Level level() const { return std::get<2>(GetParam()); }
+};
+
+INSTANTIATE_TEST_SUITE_P(
+    All,
+    DataControlsClipboardUtilsDismissTest,
+    testing::Combine(testing::Bool(),
+                     testing::Bool(),
+                     testing::Values(data_controls::Rule::Level::kBlock,
+                                     data_controls::Rule::Level::kWarn)));
+
+IN_PROC_BROWSER_TEST_P(DataControlsClipboardUtilsDismissTest,
+                       Paste_DismissWithEsc) {
+  active_user_test_mixin_->SetFakeCookieValue();
+
+  base::RunLoop run_loop;
+  auto event_validator = event_report_validator_helper_->CreateValidator();
+  event_validator.SetDoneClosure(run_loop.QuitClosure());
+
+  chrome::cros::reporting::proto::DlpSensitiveDataEvent expected_event;
+  if (use_workspace_urls()) {
+    expected_event.set_web_app_signed_in_account(kContentAreaUser1);
+  }
+  expected_event.set_url(test_url_1());
+  expected_event.set_tab_url(test_url_1());
+  expected_event.set_source("CLIPBOARD");
+  expected_event.set_destination(test_url_1());
+  expected_event.set_content_type("text/plain");
+  expected_event.set_content_size(1234);
+  expected_event.set_trigger(chrome::cros::reporting::proto::
+                                 DataTransferEventTrigger::WEB_CONTENT_UPLOAD);
+  expected_event.set_event_result(
+      level() == data_controls::Rule::Level::kBlock
+          ? chrome::cros::reporting::proto::EventResult::EVENT_RESULT_BLOCKED
+          : chrome::cros::reporting::proto::EventResult::EVENT_RESULT_WARNED);
+
+  ::chrome::cros::reporting::proto::TriggeredRuleInfo triggered_rule;
+  triggered_rule.set_rule_id(1234);
+  triggered_rule.set_rule_name("rule_name");
+
+  *expected_event.add_triggered_rule_info() = triggered_rule;
+  expected_event.set_profile_identifier(
+      browser()->GetProfile()->GetPath().AsUTF8Unsafe());
+  expected_event.set_profile_user_name(kUserName);
+
+  event_validator.ExpectSensitiveDataEvent(std::move(expected_event));
+
+  data_controls::SetDataControls(
+      browser()->GetProfile()->GetPrefs(),
+      {base::StringPrintf(R"({
+                 "name": "rule_name",
+                 "rule_id": "1234",
+                 "destinations": {
+                   "urls": ["google.com", "not.workspace.com"]
+                 },
+                 "restrictions": [
+                   {"class": "CLIPBOARD", "level": "%s"}
+                 ]
+               })",
+                          data_controls::Rule::LevelToString(level()))},
+      machine_scope());
+  data_controls::DesktopDataControlsDialogTestHelper helper(
+      level() == data_controls::Rule::Level::kBlock
+          ? data_controls::DataControlsDialog::Type::kClipboardPasteBlock
+          : data_controls::DataControlsDialog::Type::kClipboardPasteWarn);
+
+  base::test::TestFuture<std::optional<content::ClipboardPasteData>> future;
+  auto source = content::ClipboardEndpoint(std::nullopt);
+  auto destination = CreateURLClipboardEndpoint(test_url_1());
+  ui::ClipboardMetadata metadata = {
+      .size = 1234,
+      .format_type = ui::ClipboardFormatType::PlainTextType(),
+  };
+  EXPECT_TRUE(IsPastePolicyCheckRequired(source, destination, metadata));
+  PasteIfAllowedByPolicy(source, destination, metadata,
+                         MakeClipboardPasteData("text", "image", {}),
+                         future.GetCallback());
+
+  helper.WaitForDialogToInitialize();
+  if (level() == data_controls::Rule::Level::kWarn) {
+    EXPECT_FALSE(future.IsReady());
+  }
+  helper.DismissWithEsc();
+  helper.WaitForDialogToClose();
+
+  auto paste_data = future.Get();
+  EXPECT_FALSE(paste_data);
+  run_loop.Run();
+}
+
 // ChromeOS requires extra boilerplate to run this test, and since copy-pasting
 // between profiles on ChromeOS isn't a meaningful test it is simply omitted
 // from running this.
Loading diff…

Regression Test / PoC

shipped with the fix
diff --git a/chrome/browser/enterprise/data_controls/desktop_data_controls_dialog_browsertest.cc b/chrome/browser/enterprise/data_controls/desktop_data_controls_dialog_browsertest.cc
index f719f6f..afe92d89 100644
--- a/chrome/browser/enterprise/data_controls/desktop_data_controls_dialog_browsertest.cc
+++ b/chrome/browser/enterprise/data_controls/desktop_data_controls_dialog_browsertest.cc
@@ -17,8 +17,12 @@
 #include "chrome/test/base/in_process_browser_test.h"
 #include "content/public/test/browser_test.h"
 #include "content/public/test/browser_test_utils.h"
+#include "ui/base/accelerators/accelerator.h"
 #include "ui/base/mojom/dialog_button.mojom.h"
+#include "ui/events/keycodes/keyboard_codes.h"
 #include "ui/views/controls/webview/web_dialog_view.h"
+#include "ui/views/widget/widget.h"
+#include "ui/views/window/dialog_client_view.h"
 #include "ui/web_dialogs/test/test_web_dialog_delegate.h"
 
 namespace data_controls {
@@ -62,6 +66,7 @@
 
     ASSERT_EQ(delegate->GetDefaultDialogButton(),
               static_cast<int>(ui::mojom::DialogButton::kOk));
+    ASSERT_FALSE(delegate->EscShouldCancelDialog());
 
     ASSERT_FALSE(delegates_.contains(dialog));
     ASSERT_FALSE(dialog_close_loops_.contains(dialog));
@@ -254,4 +259,74 @@
   CloseDialogsAndWait();
 }
 
+class DesktopDataControlsDialogDismissTest
+    : public DesktopDataControlsDialogTest,
+      public testing::WithParamInterface<DataControlsDialog::Type> {};
+
+INSTANTIATE_TEST_SUITE_P(
+    All,
+    DesktopDataControlsDialogDismissTest,
+    testing::Values(DataControlsDialog::Type::kClipboardPasteWarn,
+                    DataControlsDialog::Type::kClipboardCopyWarn,
+                    DataControlsDialog::Type::kClipboardPasteBlock,
+                    DataControlsDialog::Type::kClipboardCopyBlock,
+                    DataControlsDialog::Type::kClipboardDragBlock));
+
+IN_PROC_BROWSER_TEST_P(DesktopDataControlsDialogDismissTest, EscDoesNotBypass) {
+  base::test::TestFuture<bool> was_bypassed;
+  DesktopDataControlsDialogFactory::GetInstance()->ShowDialogIfNeeded(
+      browser()->GetTabStripModel()->GetActiveWebContents(), GetParam(),
+      was_bypassed.GetCallback());
+
+  ASSERT_EQ(constructor_called_count_, 1u);
+  auto* dialog = delegates_.begin()->first;
+  auto* delegate = delegates_.begin()->second;
+
+  // Closing the dialog with ESC should never bypass the warning or block.
+  base::SingleThreadTaskRunner::GetCurrentDefault()->PostTask(
+      FROM_HERE, base::BindOnce(
+                     [](views::DialogDelegate* delegate) {
+                       views::DialogClientView* dialog_client_view =
+                           delegate->GetDialogClientView();
+                       ASSERT_TRUE(dialog_client_view);
+                       dialog_client_view->AcceleratorPressed(
+                           ui::Accelerator(ui::VKEY_ESCAPE, ui::EF_NONE));
+                     },
+                     base::Unretained(delegate)));
+
+  dialog_close_loops_[dialog]->Run();
+
+  ASSERT_TRUE(was_bypassed.IsReady());
+  EXPECT_FALSE(was_bypassed.Get());
+}
+
+IN_PROC_BROWSER_TEST_P(DesktopDataControlsDialogDismissTest,
+                       CloseWithDialogButton) {
+  base::test::TestFuture<bool> was_bypassed;
+  DesktopDataControlsDialogFactory::GetInstance()->ShowDialogIfNeeded(
+      browser()->GetTabStripModel()->GetActiveWebContents(), GetParam(),
+      was_bypassed.GetCallback());
+
+  ASSERT_EQ(constructor_called_count_, 1u);
+  auto* dialog = delegates_.begin()->first;
+  auto* delegate = delegates_.begin()->second;
+
+  const bool can_bypass =
+      GetParam() == DataControlsDialog::Type::kClipboardPasteWarn ||
+      GetParam() == DataControlsDialog::Type::kClipboardCopyWarn;
+
+  // For warning dialogs, clicking the "cancel" button bypasses the warning.
+  // For block dialogs, clicking the "OK" button acknowledges the block.
+  base::SingleThreadTaskRunner::GetCurrentDefault()->PostTask(
+      FROM_HERE,
+      base::BindOnce(can_bypass ? &views::DialogDelegate::CancelDialog
+                                : &views::DialogDelegate::AcceptDialog,
+                     base::Unretained(delegate)));
+
+  dialog_close_loops_[dialog]->Run();
+
+  ASSERT_TRUE(was_bypassed.IsReady());
+  EXPECT_EQ(was_bypassed.Get(), can_bypass);
+}
+
 }  // namespace data_controls
diff --git a/chrome/browser/enterprise/data_protection/data_protection_clipboard_utils_browsertest.cc b/chrome/browser/enterprise/data_protection/data_protection_clipboard_utils_browsertest.cc
index b4a22d2..285966de 100644
--- a/chrome/browser/enterprise/data_protection/data_protection_clipboard_utils_browsertest.cc
+++ b/chrome/browser/enterprise/data_protection/data_protection_clipboard_utils_browsertest.cc
@@ -7,6 +7,7 @@
 #include "base/functional/bind.h"
 #include "base/functional/callback_helpers.h"
 #include "base/run_loop.h"
+#include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/test/bind.h"
 #include "base/test/metrics/histogram_tester.h"
@@ -40,6 +41,7 @@
 #include "components/enterprise/connectors/core/reporting_test_utils.h"
 #include "components/enterprise/data_controls/content/browser/last_replaced_clipboard_data.h"
 #include "components/enterprise/data_controls/core/browser/features.h"
+#include "components/enterprise/data_controls/core/browser/rule.h"
 #include "components/enterprise/data_controls/core/browser/test_utils.h"
 #include "components/policy/core/common/cloud/cloud_policy_client.h"
 #include "components/policy/core/common/cloud/cloud_policy_core.h"
@@ -95,11 +97,10 @@
 // Tests for functions and classes declared in data_protection_clipboard_utils.h
 // For browser tests that test data protection integration with Chrome's
 // clipboard logic, see clipboard_browsertests.cc
-class DataControlsClipboardUtilsBrowserTest
-    : public MixinBasedInProcessBrowserTest,
-      public testing::WithParamInterface<std::tuple<bool, bool>> {
+class DataControlsClipboardUtilsBrowserTestBase
+    : public MixinBasedInProcessBrowserTest {
  public:
-  DataControlsClipboardUtilsBrowserTest() {
+  DataControlsClipboardUtilsBrowserTestBase() {
     std::vector<base::test::FeatureRef> enabled_features = {
         data_controls::kDataControlsSearchWith,
         enterprise_connectors::kContentAnalysisClipboardCopy,
@@ -117,7 +118,7 @@
     ui::TestClipboard::CreateForCurrentThread();
   }
 
-  ~DataControlsClipboardUtilsBrowserTest() override {
+  ~DataControlsClipboardUtilsBrowserTestBase() override {
     ui::Clipboard::DestroyClipboardForCurrentThread();
   }
 
@@ -201,9 +202,9 @@
 #endif
   }
 
-  bool machine_scope() const { return std::get<0>(GetParam()); }
+  virtual bool machine_scope() const = 0;
 
-  bool use_workspace_urls() const { return std::get<1>(GetParam()); }
+  virtual bool use_workspace_urls() const = 0;
 
   const char* test_url_0() const {
     return use_workspace_urls() ? kWorkspaceUrlForUser0 : kNonWorkspaceUrl;
@@ -261,6 +262,14 @@
       active_user_test_mixin_;
 };
 
+class DataControlsClipboardUtilsBrowserTest
+    : public DataControlsClipboardUtilsBrowserTestBase,
+      public testing::WithParamInterface<std::tuple<bool, bool>> {
+ public:
+  bool machine_scope() const override { return std::get<0>(GetParam()); }
+  bool use_workspace_urls() const override { return std::get<1>(GetParam()); }
+};
+
 IN_PROC_BROWSER_TEST_P(DataControlsClipboardUtilsBrowserTest,
                        CopyContentAnalysisWarningCanceled) {
   active_user_test_mixin_->SetFakeCookieValue();
@@ -680,6 +689,103 @@
   run_loop.Run();
 }
 
+class DataControlsClipboardUtilsDismissTest
+    : public DataControlsClipboardUtilsBrowserTestBase,
+      public testing::WithParamInterface<
+          std::tuple<bool, bool, data_controls::Rule::Level>> {
+ public:
+  bool machine_scope() const override { return std::get<0>(GetParam()); }
+  bool use_workspace_urls() const override { return std::get<1>(GetParam()); }
+  data_controls::Rule::Level level() const { return std::get<2>(GetParam()); }
+};
+
+INSTANTIATE_TEST_SUITE_P(
+    All,
+    DataControlsClipboardUtilsDismissTest,
+    testing::Combine(testing::Bool(),
+                     testing::Bool(),
+                     testing::Values(data_controls::Rule::Level::kBlock,
+                                     data_controls::Rule::Level::kWarn)));
+
+IN_PROC_BROWSER_TEST_P(DataControlsClipboardUtilsDismissTest,
+                       Paste_DismissWithEsc) {
+  active_user_test_mixin_->SetFakeCookieValue();
+
+  base::RunLoop run_loop;
+  auto event_validator = event_report_validator_helper_->CreateValidator();
+  event_validator.SetDoneClosure(run_loop.QuitClosure());
+
+  chrome::cros::reporting::proto::DlpSensitiveDataEvent expected_event;
+  if (use_workspace_urls()) {
+    expected_event.set_web_app_signed_in_account(kContentAreaUser1);
+  }
+  expected_event.set_url(test_url_1());
+  expected_event.set_tab_url(test_url_1());
+  expected_event.set_source("CLIPBOARD");
+  expected_event.set_destination(test_url_1());
+  expected_event.set_content_type("text/plain");
+  expected_event.set_content_size(1234);
+  expected_event.set_trigger(chrome::cros::reporting::proto::
+                                 DataTransferEventTrigger::WEB_CONTENT_UPLOAD);
+  expected_event.set_event_result(
+      level() == data_controls::Rule::Level::kBlock
+          ? chrome::cros::reporting::proto::EventResult::EVENT_RESULT_BLOCKED
+          : chrome::cros::reporting::proto::EventResult::EVENT_RESULT_WARNED);
+
+  ::chrome::cros::reporting::proto::TriggeredRuleInfo triggered_rule;
+  triggered_rule.set_rule_id(1234);
+  triggered_rule.set_rule_name("rule_name");
+
+  *expected_event.add_triggered_rule_info() = triggered_rule;
+  expected_event.set_profile_identifier(
+      browser()->GetProfile()->GetPath().AsUTF8Unsafe());
+  expected_event.set_profile_user_name(kUserName);
+
+  event_validator.ExpectSensitiveDataEvent(std::move(expected_event));
+
+  data_controls::SetDataControls(
+      browser()->GetProfile()->GetPrefs(),
+      {base::StringPrintf(R"({
+                 "name": "rule_name",
+                 "rule_id": "1234",
+                 "destinations": {
+                   "urls": ["google.com", "not.workspace.com"]
+                 },
+                 "restrictions": [
+                   {"class": "CLIPBOARD", "level": "%s"}
+                 ]
+               })",
+                          data_controls::Rule::LevelToString(level()))},
+      machine_scope());
+  data_controls::DesktopDataControlsDialogTestHelper helper(
+      level() == data_controls::Rule::Level::kBlock
+          ? data_controls::DataControlsDialog::Type::kClipboardPasteBlock
+          : data_controls::DataControlsDialog::Type::kClipboardPasteWarn);
+
+  base::test::TestFuture<std::optional<content::ClipboardPasteData>> future;
+  auto source = content::ClipboardEndpoint(std::nullopt);
+  auto destination = CreateURLClipboardEndpoint(test_url_1());
+  ui::ClipboardMetadata metadata = {
+      .size = 1234,
+      .format_type = ui::ClipboardFormatType::PlainTextType(),
+  };
+  EXPECT_TRUE(IsPastePolicyCheckRequired(source, destination, metadata));
+  PasteIfAllowedByPolicy(source, destination, metadata,
+                         MakeClipboardPasteData("text", "image", {}),
+                         future.GetCallback());
+
+  helper.WaitForDialogToInitialize();
+  if (level() == data_controls::Rule::Level::kWarn) {
+    EXPECT_FALSE(future.IsReady());
+  }
+  helper.DismissWithEsc();
+  helper.WaitForDialogToClose();
+
+  auto paste_data = future.Get();
+  EXPECT_FALSE(paste_data);
+  run_loop.Run();
+}
+
 // ChromeOS requires extra boilerplate to run this test, and since copy-pasting
 // between profiles on ChromeOS isn't a meaningful test it is simply omitted
 // from running this.
Loading diff…

Original Bug Report

reported by vm...@google.com

Escape key potentially bypasses Enterprise Data Controls clipboard warning dialogs

Project Fortify, an experimental security project, has identified the following potential security issue. If you’re a feature owner CC-ed on this bug, please do your best to review these reports. Please see https://chromium.googlesource.com/chromium/src/+/main/docs/security/ai-generated-security-bugs-faq.md for more information.

Overview: In desktop Enterprise Data Controls warning dialogs, button roles are configured so that the Cancel slot carries the bypass action (‘Paste anyway’ / ‘Copy anyway’) while the OK slot cancels the operation. Because the dialog delegate does not show a close button or register a close callback, pressing Escape triggers CancelDialog() without input protection, unintentionally executing the bypass action and exposing protected clipboard data. This behavior allows sensitive corporate data to be released to a webpage when a user attempts to dismiss the dialog or via unhandled keyboard event redispatch.

Affected files:

  • chrome/browser/enterprise/data_controls/desktop_data_controls_dialog.cc
  • chrome/browser/enterprise/data_protection/data_protection_clipboard_utils.cc

Estimated timestamp from git blame: 2024-06-27

Potential Issue Summary

In desktop Enterprise Data Controls warning dialogs (such as clipboard copy/paste warnings), pressing the Escape key triggers the bypass action (bypassed=true), releasing enterprise-protected clipboard data and audit-logging the action as an intentional user bypass.

Root Cause Analysis

  1. Inverted Button Configuration: In chrome/browser/enterprise/data_controls/desktop_data_controls_dialog.cc:52-111, warning dialogs intentionally invert button semantics to present ‘Don’t paste’ / ‘Don’t copy’ on the default prominent OK button and ‘Paste anyway’ / ‘Copy anyway’ on the tonal Cancel button:

    case DataControlsDialog::Type::kClipboardPasteWarn:
      SetButtons(static_cast<int>(ui::mojom::DialogButton::kCancel) |
                 static_cast<int>(ui::mojom::DialogButton::kOk));
      SetButtonLabel(ui::mojom::DialogButton::kOk,
                     l10n_util::GetStringUTF16(IDS_DATA_CONTROLS_PASTE_WARN_CANCEL_BUTTON)); // "Don't paste"
      SetButtonStyle(ui::mojom::DialogButton::kCancel, ui::ButtonStyle::kTonal);
      SetButtonLabel(ui::mojom::DialogButton::kCancel,
                     l10n_util::GetStringUTF16(IDS_DATA_CONTROLS_PASTE_WARN_CONTINUE_BUTTON)); // "Paste anyway"
      SetDefaultButton(static_cast<int>(ui::mojom::DialogButton::kOk));
    
  2. Escape Key Routes to CancelDialog(): DataControlsDialogDelegate::ShouldShowCloseButton() returns false (desktop_data_controls_dialog.cc:202), and no close callback is registered. Consequently, DialogDelegate::EscShouldCancelDialog() (ui/views/window/dialog_delegate.cc:361-367) returns true. When Escape is pressed, DialogClientView::AcceleratorPressed (ui/views/window/dialog_client_view.cc:251-266) calls delegate->CancelDialog() without checking input_protector_->IsPossiblyUnintendedInteraction().

  3. Cancel Reason Triggers Policy Bypass: DialogDelegate::CancelDialog() (ui/views/window/dialog_delegate.cc:589-612) closes the widget with views::Widget::ClosedReason::kCancelButtonClicked. Through Widget::MakeCloseSynchronous (desktop_data_controls_dialog.cc:282-283), this reason is handled by DesktopDataControlsDialog::CloseDialog (desktop_data_controls_dialog.cc:286-292):

    void DesktopDataControlsDialog::CloseDialog(views::Widget::ClosedReason reason) {
      if (reason == views::Widget::ClosedReason::kCancelButtonClicked) {
        OnDialogButtonClicked(/*bypassed=*/true);
      } else {
        OnDialogButtonClicked(/*bypassed=*/false);
      }
      ...
    }
    
  4. Data Exfiltration: OnDialogButtonClicked(/*bypassed=*/true) calls OnDataControlsPasteWarning (chrome/browser/enterprise/data_protection/data_protection_clipboard_utils.cc:584-614), which passes the unredacted clipboard data to the requesting web page and logs MaybeReportDataControlsPaste(..., /*bypassed=*/true).

Potential Attack Scenarios

(Note: These are potential steps deduced from static analysis; automated dynamic proof-of-concept execution was not performed.)

  • Scenario A (Direct User Dismissal):

    1. An enterprise user has sensitive corporate data on the clipboard.
    2. On visiting a webpage, a paste operation triggers the Data Controls warn dialog.
    3. The user presses Escape expecting to dismiss and cancel the paste request.
    4. The dialog closes with bypassed=true, unexpectedly transferring the clipboard contents to the web page.
  • Scenario B (Webpage Unhandled-Key Redispatch on Aura platforms):

    1. An attacker-controlled site obtains clipboard-read permission and prompts the user with an ‘Esc to close’ UI banner.
    2. On receiving the Escape keydown event, the site requests navigator.clipboard.readText() and yields/delays so the browser displays the child modal dialog.
    3. The unhandled Escape key event is redispatched through RenderWidgetHostImpl::OnKeyboardEventAck -> UnhandledKeyboardEventHandler::HandleKeyboardEvent -> FocusManager::ProcessAccelerator(VKEY_ESCAPE).
    4. The dialog’s Escape accelerator executes CancelDialog(), immediately fulfilling the readText() promise with the corporate clipboard content.

Suggested Fix

  1. Ensure that dismissal via the Escape key does not map to bypassed=true. This can be achieved by calling set_esc_should_cancel_dialog_override(false) on the DialogDelegate or setting an explicit close callback via SetCloseCallback mapped to bypassed=false.
  2. Apply InputEventActivationProtector checks to accelerator-based dismissal paths if they trigger non-default dialog actions.

Evaluated with Chrome root at commit: c6718ebbf2c677c6f63691e28c7801c563f6b19c


Results so far have been promising, but there can be wrong deductions. Feel free to adjust as follows:

  • If you are familiar with the severity guidelines, you may adjust the severity.
  • If this is a false positive, and there’s no work to be done, please close as WAI.
  • If there is work to do here but not a vulnerability, please change the issue type to Task/Bug/FR.

Data from false positives will be used to improve accuracy over time. And please feel free to reach out to me directly if you have concerns or feedback on the project.

Raised in root component due to access or custom field issues on 1208119

View on issue tracker