CVE-2026-106352
Overview
Changed Functions
| Function | Change | Notes |
|---|---|---|
DesktopDataControlsDialogDismissTestchrome/browser/enterprise/data_controls/desktop_data_controls_dialog_browsertest.cc |
modified | |
IN_PROC_BROWSER_TEST_Pchrome/browser/enterprise/data_controls/desktop_data_controls_dialog_browsertest.cc |
modified | |
DataControlsClipboardUtilsBrowserTestchrome/browser/enterprise/data_protection/data_protection_clipboard_utils_browsertest.cc |
modified | |
DataControlsClipboardUtilsBrowserTestBasechrome/browser/enterprise/data_protection/data_protection_clipboard_utils_browsertest.cc |
modified | |
DataControlsClipboardUtilsBrowserTestchrome/browser/enterprise/data_protection/data_protection_clipboard_utils_browsertest.cc |
modified | |
DataControlsClipboardUtilsBrowserTestBasechrome/browser/enterprise/data_protection/data_protection_clipboard_utils_browsertest.cc |
modified | |
DataControlsClipboardUtilsDismissTestchrome/browser/enterprise/data_protection/data_protection_clipboard_utils_browsertest.cc |
modified |
Files Changed
chrome/browser/enterprise/data_controls/desktop_data_controls_dialog_browsertest.ccchrome/browser/enterprise/data_controls/desktop_data_controls_dialog.ccchrome/browser/enterprise/data_controls/desktop_data_controls_dialog_test_helper.ccchrome/browser/enterprise/data_controls/desktop_data_controls_dialog_test_helper.hchrome/browser/enterprise/data_protection/data_protection_clipboard_utils_browsertest.cc
Patch
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.
Regression Test / PoC
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.
Original Bug Report
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.ccchrome/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
-
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)); -
Escape Key Routes to
CancelDialog():DataControlsDialogDelegate::ShouldShowCloseButton()returnsfalse(desktop_data_controls_dialog.cc:202), and no close callback is registered. Consequently,DialogDelegate::EscShouldCancelDialog()(ui/views/window/dialog_delegate.cc:361-367) returnstrue. When Escape is pressed,DialogClientView::AcceleratorPressed(ui/views/window/dialog_client_view.cc:251-266) callsdelegate->CancelDialog()without checkinginput_protector_->IsPossiblyUnintendedInteraction(). -
Cancel Reason Triggers Policy Bypass:
DialogDelegate::CancelDialog()(ui/views/window/dialog_delegate.cc:589-612) closes the widget withviews::Widget::ClosedReason::kCancelButtonClicked. ThroughWidget::MakeCloseSynchronous(desktop_data_controls_dialog.cc:282-283), this reason is handled byDesktopDataControlsDialog::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); } ... } -
Data Exfiltration:
OnDialogButtonClicked(/*bypassed=*/true)callsOnDataControlsPasteWarning(chrome/browser/enterprise/data_protection/data_protection_clipboard_utils.cc:584-614), which passes the unredacted clipboard data to the requesting web page and logsMaybeReportDataControlsPaste(..., /*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):
- An enterprise user has sensitive corporate data on the clipboard.
- On visiting a webpage, a paste operation triggers the Data Controls warn dialog.
- The user presses Escape expecting to dismiss and cancel the paste request.
- The dialog closes with
bypassed=true, unexpectedly transferring the clipboard contents to the web page.
-
Scenario B (Webpage Unhandled-Key Redispatch on Aura platforms):
- An attacker-controlled site obtains clipboard-read permission and prompts the user with an ‘Esc to close’ UI banner.
- On receiving the Escape
keydownevent, the site requestsnavigator.clipboard.readText()and yields/delays so the browser displays the child modal dialog. - The unhandled Escape key event is redispatched through
RenderWidgetHostImpl::OnKeyboardEventAck->UnhandledKeyboardEventHandler::HandleKeyboardEvent->FocusManager::ProcessAccelerator(VKEY_ESCAPE). - The dialog’s Escape accelerator executes
CancelDialog(), immediately fulfilling thereadText()promise with the corporate clipboard content.
Suggested Fix
- Ensure that dismissal via the Escape key does not map to
bypassed=true. This can be achieved by callingset_esc_should_cancel_dialog_override(false)on theDialogDelegateor setting an explicit close callback viaSetCloseCallbackmapped tobypassed=false. - Apply
InputEventActivationProtectorchecks 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