CVE-2026-13901
Overview
Changed Functions
| Function | Change | Notes |
|---|---|---|
ifcontent/browser/media/captured_surface_controller_permission_manager_unittest.cc |
modified |
Files Changed
content/browser/fenced_frame/fenced_frame_browsertest.cccontent/browser/file_system_access/file_system_access_manager_impl.cccontent/browser/font_access/font_access_manager.cccontent/browser/media/captured_surface_controller_permission_manager_unittest.cccontent/browser/renderer_host/frame_tree_browsertest.cccontent/browser/renderer_host/frame_tree_node.hcontent/browser/renderer_host/navigation_controller_history_intervention_browsertest.cc
Patch
From 4bf8918d8230c86e37900d74004f9213e21eff18 Mon Sep 17 00:00:00 2001
From: Frank Liberato <liberato@chromium.org>
Date: Wed, 27 May 2026 17:49:55 -0700
Subject: [PATCH] Check UpdateUserActivationState more often.
This CL adds [[nodiscard]] to catch future calls to Update..State()
that ignore the return. Existing callers are updated to use the
return value properly if possible, CHECK() if it's supposed to be
guaranteed to succeed, or (in a few cases) ignore it to prevent
going down a garden path.
Tests try to EXPECT_ calls to it.
Bug: 503585173
Change-Id: Id3462092275fa5b3e85688918f333f67a792f8dd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7833070
Reviewed-by: Thomas Guilbert <tguilbert@chromium.org>
Reviewed-by: Xiaochen Zhou <xiaochenzh@chromium.org>
Reviewed-by: Fergal Daly <fergal@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Commit-Queue: Frank Liberato <liberato@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1637357}
---
diff --git a/content/browser/fenced_frame/fenced_frame_browsertest.cc b/content/browser/fenced_frame/fenced_frame_browsertest.cc
index f346077..56ebed0 100644
--- a/content/browser/fenced_frame/fenced_frame_browsertest.cc
+++ b/content/browser/fenced_frame/fenced_frame_browsertest.cc
@@ -4971,9 +4971,9 @@
};
auto Activate = [](FrameTreeNode* node) {
- node->UpdateUserActivationState(
+ EXPECT_TRUE(node->UpdateUserActivationState(
blink::mojom::UserActivationUpdateType::kNotifyActivation,
- blink::mojom::UserActivationNotificationType::kTest);
+ blink::mojom::UserActivationNotificationType::kTest));
};
auto EXPECT_STICKY = [&nodes](std::vector<bool> should_be_activated) {
@@ -5056,9 +5056,9 @@
};
auto Consume = [](FrameTreeNode* node) {
- node->UpdateUserActivationState(
+ EXPECT_TRUE(node->UpdateUserActivationState(
blink::mojom::UserActivationUpdateType::kConsumeTransientActivation,
- blink::mojom::UserActivationNotificationType::kTest);
+ blink::mojom::UserActivationNotificationType::kTest));
};
auto EXPECT_TRANSIENT = [&nodes](std::vector<bool> should_be_activated) {
diff --git a/content/browser/file_system_access/file_system_access_manager_impl.cc b/content/browser/file_system_access/file_system_access_manager_impl.cc
index bf53abc..e40ce0c 100644
--- a/content/browser/file_system_access/file_system_access_manager_impl.cc
+++ b/content/browser/file_system_access/file_system_access_manager_impl.cc
@@ -621,9 +621,16 @@
->browser()
->IsTransientActivationRequiredForShowFileOrDirectoryPicker(
WebContents::FromRenderFrameHost(rfh))) {
- FrameTreeNode::From(rfh)->UpdateUserActivationState(
- blink::mojom::UserActivationUpdateType::kConsumeTransientActivation,
- blink::mojom::UserActivationNotificationType::kNone);
+ if (!FrameTreeNode::From(rfh)->UpdateUserActivationState(
+ blink::mojom::UserActivationUpdateType::kConsumeTransientActivation,
+ blink::mojom::UserActivationNotificationType::kNone)) {
+ std::move(callback).Run(
+ file_system_access_error::FromStatus(
+ FileSystemAccessStatus::kPermissionDenied,
+ "User activation required."),
+ std::vector<blink::mojom::FileSystemAccessEntryPtr>());
+ return;
+ }
}
// Don't show the file picker if there is an already active file picker for
diff --git a/content/browser/font_access/font_access_manager.cc b/content/browser/font_access/font_access_manager.cc
index 7c21734..8b0e907 100644
--- a/content/browser/font_access/font_access_manager.cc
+++ b/content/browser/font_access/font_access_manager.cc
@@ -128,9 +128,14 @@
base::ReadOnlySharedMemoryRegion());
return;
}
- rfh->frame_tree_node()->UpdateUserActivationState(
- blink::mojom::UserActivationUpdateType::kConsumeTransientActivation,
- blink::mojom::UserActivationNotificationType::kNone);
+ if (!rfh->frame_tree_node()->UpdateUserActivationState(
+ blink::mojom::UserActivationUpdateType::kConsumeTransientActivation,
+ blink::mojom::UserActivationNotificationType::kNone)) {
+ std::move(callback).Run(
+ blink::mojom::FontEnumerationStatus::kNeedsUserActivation,
+ base::ReadOnlySharedMemoryRegion());
+ return;
+ }
permission_controller->RequestPermissionFromCurrentDocument(
rfh,
diff --git a/content/browser/media/captured_surface_controller_permission_manager_unittest.cc b/content/browser/media/captured_surface_controller_permission_manager_unittest.cc
index b887d7e0..bdd1b85b7 100644
--- a/content/browser/media/captured_surface_controller_permission_manager_unittest.cc
+++ b/content/browser/media/captured_surface_controller_permission_manager_unittest.cc
@@ -3,6 +3,7 @@
// found in the LICENSE file.
#include <memory>
+#include <tuple>
#include "base/functional/bind.h"
#include "base/run_loop.h"
@@ -204,7 +205,7 @@
if (has_activation) {
rfh->SimulateUserActivation();
} else {
- rfh->frame_tree_node()->UpdateUserActivationState(
+ std::ignore = rfh->frame_tree_node()->UpdateUserActivationState(
blink::mojom::UserActivationUpdateType::kConsumeTransientActivation,
blink::mojom::UserActivationNotificationType::kTest);
}
diff --git a/content/browser/renderer_host/frame_tree_browsertest.cc b/content/browser/renderer_host/frame_tree_browsertest.cc
index 88014e3..77b119d 100644
--- a/content/browser/renderer_host/frame_tree_browsertest.cc
+++ b/content/browser/renderer_host/frame_tree_browsertest.cc
@@ -1494,9 +1494,9 @@
EXPECT_FALSE(root->HasTransientUserActivation());
// Set the user activation bits.
- root->UpdateUserActivationState(
+ EXPECT_TRUE(root->UpdateUserActivationState(
blink::mojom::UserActivationUpdateType::kNotifyActivation,
- blink::mojom::UserActivationNotificationType::kTest);
+ blink::mojom::UserActivationNotificationType::kTest));
EXPECT_TRUE(root->HasStickyUserActivation());
EXPECT_TRUE(root->HasTransientUserActivation());
@@ -1949,9 +1949,9 @@
EXPECT_FALSE(root->HasTransientUserActivation());
// Set the user activation bits.
- root->UpdateUserActivationState(
+ EXPECT_TRUE(root->UpdateUserActivationState(
blink::mojom::UserActivationUpdateType::kNotifyActivation,
- blink::mojom::UserActivationNotificationType::kTest);
+ blink::mojom::UserActivationNotificationType::kTest));
EXPECT_TRUE(root->HasStickyUserActivation());
EXPECT_TRUE(root->HasTransientUserActivation());
diff --git a/content/browser/renderer_host/frame_tree_node.h b/content/browser/renderer_host/frame_tree_node.h
index b645e0d..f6d7d73c 100644
--- a/content/browser/renderer_host/frame_tree_node.h
+++ b/content/browser/renderer_host/frame_tree_node.h
@@ -719,7 +719,7 @@
//
// The |notification_type| parameter is used for histograms, only for the case
// |update_state == kNotifyActivation|.
- bool UpdateUserActivationState(
+ [[nodiscard]] bool UpdateUserActivationState(
blink::mojom::UserActivationUpdateType update_type,
blink::mojom::UserActivationNotificationType notification_type) override;
void DidConsumeHistoryUserActivation() override;
diff --git a/content/browser/renderer_host/navigation_controller_history_intervention_browsertest.cc b/content/browser/renderer_host/navigation_controller_history_intervention_browsertest.cc
index e79b3c0..751f2b84 100644
--- a/content/browser/renderer_host/navigation_controller_history_intervention_browsertest.cc
+++ b/content/browser/renderer_host/navigation_controller_history_intervention_browsertest.cc
@@ -1165,9 +1165,9 @@
controller.GetLastCommittedEntry()->should_skip_on_back_forward_ui());
// Simulate a user gesture.
- root->UpdateUserActivationState(
+ EXPECT_TRUE(root->UpdateUserActivationState(
blink::mojom::UserActivationUpdateType::kNotifyActivation,
- blink::mojom::UserActivationNotificationType::kTest);
+ blink::mojom::UserActivationNotificationType::kTest));
// Since the last navigations refer to a different document, a user gesture
// here should not reset the skippable bit in the previous entries.
@@ -1805,9 +1805,9 @@
// A user gesture in the main frame now will lead to all same document
// entries to be marked as non-skippable.
- root->UpdateUserActivationState(
+ EXPECT_TRUE(root->UpdateUserActivationState(
blink::mojom::UserActivationUpdateType::kNotifyActivation,
- blink::mojom::UserActivationNotificationType::kTest);
+ blink::mojom::UserActivationNotificationType::kTest));
EXPECT_TRUE(root->HasStickyUserActivation());
EXPECT_TRUE(root->HasTransientUserActivation());
EXPECT_FALSE(controller.GetEntryAtIndex(0)->should_skip_on_back_forward_ui());
@@ -1839,9 +1839,9 @@
// Simulate user gesture in the main frame. Subframes creating entries without
// user gesture will not lead to the last committed entry being marked as
// skippable.
- root->UpdateUserActivationState(
+ EXPECT_TRUE(root->UpdateUserActivationState(
blink::mojom::UserActivationUpdateType::kNotifyActivation,
- blink::mojom::UserActivationNotificationType::kTest);
+ blink::mojom::UserActivationNotificationType::kTest));
EXPECT_TRUE(root->HasStickyUserActivation());
EXPECT_TRUE(root->HasTransientUserActivation());
Regression Test / PoC
diff --git a/content/browser/fenced_frame/fenced_frame_browsertest.cc b/content/browser/fenced_frame/fenced_frame_browsertest.cc
index f346077..56ebed0 100644
--- a/content/browser/fenced_frame/fenced_frame_browsertest.cc
+++ b/content/browser/fenced_frame/fenced_frame_browsertest.cc
@@ -4971,9 +4971,9 @@
};
auto Activate = [](FrameTreeNode* node) {
- node->UpdateUserActivationState(
+ EXPECT_TRUE(node->UpdateUserActivationState(
blink::mojom::UserActivationUpdateType::kNotifyActivation,
- blink::mojom::UserActivationNotificationType::kTest);
+ blink::mojom::UserActivationNotificationType::kTest));
};
auto EXPECT_STICKY = [&nodes](std::vector<bool> should_be_activated) {
@@ -5056,9 +5056,9 @@
};
auto Consume = [](FrameTreeNode* node) {
- node->UpdateUserActivationState(
+ EXPECT_TRUE(node->UpdateUserActivationState(
blink::mojom::UserActivationUpdateType::kConsumeTransientActivation,
- blink::mojom::UserActivationNotificationType::kTest);
+ blink::mojom::UserActivationNotificationType::kTest));
};
auto EXPECT_TRANSIENT = [&nodes](std::vector<bool> should_be_activated) {
diff --git a/content/browser/media/captured_surface_controller_permission_manager_unittest.cc b/content/browser/media/captured_surface_controller_permission_manager_unittest.cc
index b887d7e0..bdd1b85b7 100644
--- a/content/browser/media/captured_surface_controller_permission_manager_unittest.cc
+++ b/content/browser/media/captured_surface_controller_permission_manager_unittest.cc
@@ -3,6 +3,7 @@
// found in the LICENSE file.
#include <memory>
+#include <tuple>
#include "base/functional/bind.h"
#include "base/run_loop.h"
@@ -204,7 +205,7 @@
if (has_activation) {
rfh->SimulateUserActivation();
} else {
- rfh->frame_tree_node()->UpdateUserActivationState(
+ std::ignore = rfh->frame_tree_node()->UpdateUserActivationState(
blink::mojom::UserActivationUpdateType::kConsumeTransientActivation,
blink::mojom::UserActivationNotificationType::kTest);
}
diff --git a/content/browser/renderer_host/frame_tree_browsertest.cc b/content/browser/renderer_host/frame_tree_browsertest.cc
index 88014e3..77b119d 100644
--- a/content/browser/renderer_host/frame_tree_browsertest.cc
+++ b/content/browser/renderer_host/frame_tree_browsertest.cc
@@ -1494,9 +1494,9 @@
EXPECT_FALSE(root->HasTransientUserActivation());
// Set the user activation bits.
- root->UpdateUserActivationState(
+ EXPECT_TRUE(root->UpdateUserActivationState(
blink::mojom::UserActivationUpdateType::kNotifyActivation,
- blink::mojom::UserActivationNotificationType::kTest);
+ blink::mojom::UserActivationNotificationType::kTest));
EXPECT_TRUE(root->HasStickyUserActivation());
EXPECT_TRUE(root->HasTransientUserActivation());
@@ -1949,9 +1949,9 @@
EXPECT_FALSE(root->HasTransientUserActivation());
// Set the user activation bits.
- root->UpdateUserActivationState(
+ EXPECT_TRUE(root->UpdateUserActivationState(
blink::mojom::UserActivationUpdateType::kNotifyActivation,
- blink::mojom::UserActivationNotificationType::kTest);
+ blink::mojom::UserActivationNotificationType::kTest));
EXPECT_TRUE(root->HasStickyUserActivation());
EXPECT_TRUE(root->HasTransientUserActivation());
diff --git a/content/browser/renderer_host/navigation_controller_history_intervention_browsertest.cc b/content/browser/renderer_host/navigation_controller_history_intervention_browsertest.cc
index e79b3c0..751f2b84 100644
--- a/content/browser/renderer_host/navigation_controller_history_intervention_browsertest.cc
+++ b/content/browser/renderer_host/navigation_controller_history_intervention_browsertest.cc
@@ -1165,9 +1165,9 @@
controller.GetLastCommittedEntry()->should_skip_on_back_forward_ui());
// Simulate a user gesture.
- root->UpdateUserActivationState(
+ EXPECT_TRUE(root->UpdateUserActivationState(
blink::mojom::UserActivationUpdateType::kNotifyActivation,
- blink::mojom::UserActivationNotificationType::kTest);
+ blink::mojom::UserActivationNotificationType::kTest));
// Since the last navigations refer to a different document, a user gesture
// here should not reset the skippable bit in the previous entries.
@@ -1805,9 +1805,9 @@
// A user gesture in the main frame now will lead to all same document
// entries to be marked as non-skippable.
- root->UpdateUserActivationState(
+ EXPECT_TRUE(root->UpdateUserActivationState(
blink::mojom::UserActivationUpdateType::kNotifyActivation,
- blink::mojom::UserActivationNotificationType::kTest);
+ blink::mojom::UserActivationNotificationType::kTest));
EXPECT_TRUE(root->HasStickyUserActivation());
EXPECT_TRUE(root->HasTransientUserActivation());
EXPECT_FALSE(controller.GetEntryAtIndex(0)->should_skip_on_back_forward_ui());
@@ -1839,9 +1839,9 @@
// Simulate user gesture in the main frame. Subframes creating entries without
// user gesture will not lead to the last committed entry being marked as
// skippable.
- root->UpdateUserActivationState(
+ EXPECT_TRUE(root->UpdateUserActivationState(
blink::mojom::UserActivationUpdateType::kNotifyActivation,
- blink::mojom::UserActivationNotificationType::kTest);
+ blink::mojom::UserActivationNotificationType::kTest));
EXPECT_TRUE(root->HasStickyUserActivation());
EXPECT_TRUE(root->HasTransientUserActivation());
diff --git a/content/browser/renderer_host/navigation_controller_impl_unittest.cc b/content/browser/renderer_host/navigation_controller_impl_unittest.cc
index 935d7468..cffde577 100644
--- a/content/browser/renderer_host/navigation_controller_impl_unittest.cc
+++ b/content/browser/renderer_host/navigation_controller_impl_unittest.cc
@@ -530,9 +530,9 @@
// Simulate a user gesture so that the above entry is not marked to be skipped
// on back.
- main_test_rfh()->frame_tree_node()->UpdateUserActivationState(
+ EXPECT_TRUE(main_test_rfh()->frame_tree_node()->UpdateUserActivationState(
blink::mojom::UserActivationUpdateType::kNotifyActivation,
- blink::mojom::UserActivationNotificationType::kTest);
+ blink::mojom::UserActivationNotificationType::kTest));
// Load another...
controller.LoadURL(url2, Referrer(), ui::PAGE_TRANSITION_TYPED,
@@ -1147,9 +1147,9 @@
// Simulate a user gesture so that the above entry is not marked to be skipped
// on back.
- main_test_rfh()->frame_tree_node()->UpdateUserActivationState(
+ EXPECT_TRUE(main_test_rfh()->frame_tree_node()->UpdateUserActivationState(
blink::mojom::UserActivationUpdateType::kNotifyActivation,
- blink::mojom::UserActivationNotificationType::kTest);
+ blink::mojom::UserActivationNotificationType::kTest));
const GURL kExistingURL2("http://foo/bee");
NavigationSimulator::NavigateAndCommitFromBrowser(contents(), kExistingURL2);
@@ -1194,9 +1194,9 @@
->bindings());
// Simulate a user gesture so that the above entry is not marked to be skipped
// on back.
- main_test_rfh()->frame_tree_node()->UpdateUserActivationState(
+ EXPECT_TRUE(main_test_rfh()->frame_tree_node()->UpdateUserActivationState(
blink::mojom::UserActivationUpdateType::kNotifyActivation,
- blink::mojom::UserActivationNotificationType::kTest);
+ blink::mojom::UserActivationNotificationType::kTest));
// Navigate cross-process to a second URL.
const GURL kExistingURL2("http://foo/eh");
@@ -1874,9 +1874,9 @@
// Simulate a user gesture so that the above entry is not marked to be skipped
// on back.
- main_test_rfh()->frame_tree_node()->UpdateUserActivationState(
+ EXPECT_TRUE(main_test_rfh()->frame_tree_node()->UpdateUserActivationState(
blink::mojom::UserActivationUpdateType::kNotifyActivation,
- blink::mojom::UserActivationNotificationType::kTest);
+ blink::mojom::UserActivationNotificationType::kTest));
// controller.LoadURL(kUrl2, ui::PAGE_TRANSITION_TYPED);
NavigationSimulator::NavigateAndCommitFromDocument(kUrl2, main_test_rfh());
@@ -1917,9 +1917,9 @@
// Simulate a user gesture so that the above entry is not marked to be skipped
// on back.
- main_test_rfh()->frame_tree_node()->UpdateUserActivationState(
+ EXPECT_TRUE(main_test_rfh()->frame_tree_node()->UpdateUserActivationState(
blink::mojom::UserActivationUpdateType::kNotifyActivation,
- blink::mojom::UserActivationNotificationType::kTest);
+ blink::mojom::UserActivationNotificationType::kTest));
auto forward_navigation = NavigationSimulator::CreateHistoryNavigation(
1, contents(), false /* is_renderer_initiated */);
@@ -1986,9 +1986,9 @@
// Simulate a user gesture so that the above entry is not marked to be skipped
// on back.
- main_test_rfh()->frame_tree_node()->UpdateUserActivationState(
+ EXPECT_TRUE(main_test_rfh()->frame_tree_node()->UpdateUserActivationState(
blink::mojom::UserActivationUpdateType::kNotifyActivation,
- blink::mojom::UserActivationNotificationType::kTest);
+ blink::mojom::UserActivationNotificationType::kTest));
auto forward_navigation = NavigationSimulator::CreateHistoryNavigation(
1, contents(), false /* is_renderer_initiated */);
@@ -2399,9 +2399,9 @@
navigation_entry_committed_counter_ = 0;
// Simulate a user gesture.
- main_test_rfh()->frame_tree_node()->UpdateUserActivationState(
+ EXPECT_TRUE(main_test_rfh()->frame_tree_node()->UpdateUserActivationState(
blink::mojom::UserActivationUpdateType::kNotifyActivation,
- blink::mojom::UserActivationNotificationType::kTest);
+ blink::mojom::UserActivationNotificationType::kTest));
NavigationSimulator::NavigateAndCommitFromDocument(url2, main_test_rfh());
EXPECT_EQ(1U, navigation_entry_committed_counter_);
diff --git a/content/browser/web_contents/web_contents_impl_unittest.cc b/content/browser/web_contents/web_contents_impl_unittest.cc
index bd8dc184..de35b86 100644
--- a/content/browser/web_contents/web_contents_impl_unittest.cc
+++ b/content/browser/web_contents/web_contents_impl_unittest.cc
@@ -1571,9 +1571,9 @@
// Toggle fullscreen mode on (as if initiated via IPC from renderer).
EXPECT_FALSE(contents()->IsFullscreen());
EXPECT_FALSE(fake_delegate.IsFullscreenForTabOrPending(contents()));
- main_test_rfh()->frame_tree_node()->UpdateUserActivationState(
+ EXPECT_TRUE(main_test_rfh()->frame_tree_node()->UpdateUserActivationState(
blink::mojom::UserActivationUpdateType::kNotifyActivation,
- blink::mojom::UserActivationNotificationType::kTest);
+ blink::mojom::UserActivationNotificationType::kTest));
orig_rfh->EnterFullscreen(blink::mojom::FullscreenOptions::New(),
base::BindOnce(&ExpectTrue));
EXPECT_TRUE(contents()->IsFullscreen());
@@ -1607,9 +1607,9 @@
// Make the top page fullscreen.
EXPECT_FALSE(contents()->IsFullscreen());
- main_rfh->frame_tree_node()->UpdateUserActivationState(
+ EXPECT_TRUE(main_rfh->frame_tree_node()->UpdateUserActivationState(
blink::mojom::UserActivationUpdateType::kNotifyActivation,
- blink::mojom::UserActivationNotificationType::kTest);
+ blink::mojom::UserActivationNotificationType::kTest));
main_rfh->EnterFullscreen(blink::mojom::FullscreenOptions::New(),
base::BindOnce(&ExpectTrue));
EXPECT_TRUE(contents()->IsFullscreen());
@@ -1643,9 +1643,9 @@
// Make the top page fullscreen.
EXPECT_FALSE(contents()->IsFullscreen());
- main_rfh->frame_tree_node()->UpdateUserActivationState(
+ EXPECT_TRUE(main_rfh->frame_tree_node()->UpdateUserActivationState(
blink::mojom::UserActivationUpdateType::kNotifyActivation,
- blink::mojom::UserActivationNotificationType::kTest);
+ blink::mojom::UserActivationNotificationType::kTest));
main_rfh->EnterFullscreen(blink::mojom::FullscreenOptions::New(),
base::BindOnce(&ExpectTrue));
EXPECT_TRUE(contents()->IsFullscreen());
@@ -1686,9 +1686,9 @@
// Make the subframe fullscreen.
EXPECT_FALSE(contents()->IsFullscreen());
- sub_rfh->frame_tree_node()->UpdateUserActivationState(
+ EXPECT_TRUE(sub_rfh->frame_tree_node()->UpdateUserActivationState(
blink::mojom::UserActivationUpdateType::kNotifyActivation,
- blink::mojom::UserActivationNotificationType::kTest);
+ blink::mojom::UserActivationNotificationType::kTest));
sub_rfh->EnterFullscreen(blink::mojom::FullscreenOptions::New(),
base::BindOnce(&ExpectTrue));
EXPECT_TRUE(contents()->IsFullscreen());
@@ -1728,9 +1728,9 @@
// Make the subframe fullscreen.
EXPECT_FALSE(contents()->IsFullscreen());
- sub_rfh->frame_tree_node()->UpdateUserActivationState(
+ EXPECT_TRUE(sub_rfh->frame_tree_node()->UpdateUserActivationState(
blink::mojom::UserActivationUpdateType::kNotifyActivation,
- blink::mojom::UserActivationNotificationType::kTest);
+ blink::mojom::UserActivationNotificationType::kTest));
sub_rfh->EnterFullscreen(blink::mojom::FullscreenOptions::New(),
base::BindOnce(&ExpectTrue));
EXPECT_TRUE(contents()->IsFullscreen());
@@ -1775,9 +1775,9 @@
for (int i = 0; i < 2; ++i) {
// Toggle fullscreen mode on (as if initiated via IPC from renderer).
- main_test_rfh()->frame_tree_node()->UpdateUserActivationState(
+ EXPECT_TRUE(main_test_rfh()->frame_tree_node()->UpdateUserActivationState(
blink::mojom::UserActivationUpdateType::kNotifyActivation,
- blink::mojom::UserActivationNotificationType::kTest);
+ blink::mojom::UserActivationNotificationType::kTest));
main_test_rfh()->EnterFullscreen(blink::mojom::FullscreenOptions::New(),
base::BindOnce(&ExpectTrue));
EXPECT_TRUE(contents()->IsFullscreen());
@@ -1811,9 +1811,9 @@
// Toggle fullscreen mode on (as if initiated via IPC from renderer).
EXPECT_FALSE(contents()->IsFullscreen());
EXPECT_FALSE(fake_delegate.IsFullscreenForTabOrPending(contents()));
- main_test_rfh()->frame_tree_node()->UpdateUserActivationState(
+ EXPECT_TRUE(main_test_rfh()->frame_tree_node()->UpdateUserActivationState(
blink::mojom::UserActivationUpdateType::kNotifyActivation,
- blink::mojom::UserActivationNotificationType::kTest);
+ blink::mojom::UserActivationNotificationType::kTest));
main_test_rfh()->EnterFullscreen(blink::mojom::FullscreenOptions::New(),
base::BindOnce(&ExpectTrue));
EXPECT_TRUE(contents()->IsFullscreen());
diff --git a/content/test/test_render_frame_host.cc b/content/test/test_render_frame_host.cc
index 98a1f04..51bf389 100644
--- a/content/test/test_render_frame_host.cc
+++ b/content/test/test_render_frame_host.cc
@@ -7,6 +7,7 @@
#include <algorithm>
#include <memory>
#include <optional>
+#include <tuple>
... (truncated)
Original Bug Report
Systemic User Activation bypass in WebSerial via Mojo
Pasting the entirety of http://b/495429423 but bug is about the 1 specific instance in the title.
Flapjack (go/flapjack), an LLM-powered static analysis tool, has identified the following potential security issue.
Overview: Several sensitive Mojo interfaces in the browser process fail to properly verify transient user activation before displaying UI choosers or pickers. A compromised renderer can bypass Blink’s security checks and directly invoke these interfaces to trigger dialogs without genuine user intent. When combined with an ‘Enter key holding’ social engineering attack, this can lead to unauthorized file system write access or hardware device access.
Affected files:
content/browser/file_system_access/file_system_access_manager_impl.cccontent/browser/usb/web_usb_service_impl.cccontent/browser/bluetooth/web_bluetooth_service_impl.cccontent/browser/serial/serial_service.cccontent/browser/hid/hid_service.ccchrome/browser/webshare/share_service_impl.cccontent/browser/contacts/contacts_manager_impl.cccontent/browser/presentation/presentation_service_impl.cc
Estimated timestamp from git blame: 2025-12-23
Summary
There is a potential systemic vulnerability across multiple browser-side Mojo interface implementations where transient user activation is either improperly verified or entirely unchecked before displaying sensitive UI elements (like file pickers and device choosers).
Chrome’s security architecture (User Activation v2) dictates that the browser process must be the authoritative source of truth for user activation. While Blink (the renderer) performs checks before sending these Mojo messages, a compromised renderer can bypass the Blink bindings and call the Mojo methods directly. Without robust browser-side enforcement, an attacker can forcefully display these dialogs and potentially exploit them via social engineering.
Technical Details
-
File System Access API (Flawed Check): In
content/browser/file_system_access/file_system_access_manager_impl.cc(FileSystemAccessManagerImpl::ChooseEntries), the code attempts to consume user activation:if (GetContentClient()->browser()->IsTransientActivationRequiredForShowFileOrDirectoryPicker(WebContents::FromRenderFrameHost(rfh))) { FrameTreeNode::From(rfh)->UpdateUserActivationState( blink::mojom::UserActivationUpdateType::kConsumeTransientActivation, blink::mojom::UserActivationNotificationType::kNone); } // Execution continues regardless of the return value...The method
UpdateUserActivationStatereturns aboolindicating whether transient activation was actually present and successfully consumed. However,ChooseEntriescompletely ignores this return value and proceeds to show the file picker even if it evaluates tofalse. -
Hardware APIs & Other Pickers (Missing Checks): The browser-side implementations for the following APIs do not perform any transient user activation checks before initiating their respective device choosers, relying entirely on renderer-side enforcement:
- WebUSB:
WebUsbServiceImpl::GetPermission(content/browser/usb/web_usb_service_impl.cc) - WebBluetooth:
WebBluetoothServiceImpl::RequestDeviceImpl(content/browser/bluetooth/web_bluetooth_service_impl.cc) - WebSerial:
SerialService::RequestPort(content/browser/serial/serial_service.cc) - WebHID:
HidService::RequestDevice(content/browser/hid/hid_service.cc) - Web Share:
ShareServiceImpl::Share(chrome/browser/webshare/share_service_impl.cc) - Contacts:
ContactsManagerImpl::Select(content/browser/contacts/contacts_manager_impl.cc)
- WebUSB:
Potential Attack Scenario
Note: The following steps are theoretical as the Flapjack LLM agent does not execute code, but they trace the logical flow of the vulnerability based on the codebase.
- Renderer Compromise & Social Engineering: An attacker compromises the sandboxed renderer process (e.g., via a V8 bug). The attacker’s webpage displays a lure (like a game) tricking the user into holding down the “Enter” key.
- Mojo Invocation: Bypassing Blink’s security checks, the compromised renderer directly sends a
ChooseEntriesMojo message to the browser process. It specifies aSaveFilePickerOptionstargeting the user’s Downloads folder with an attacker-controlledsuggested_name(e.g.,malware.bat). - Browser-Side Bypass: The browser process receives the message. It calls
UpdateUserActivationState(kConsumeTransientActivation). Since holding the Enter key does not constitute a valid, recent transient activation in the browser’s state, this returnsfalse. However, because the code ignores the return value, the browser proceeds. - Dialog Steals Focus: The native OS Save File Picker dialog appears and instantly intercepts window focus.
- Automatic Confirmation: Because the user is already holding the “Enter” key, the OS immediately registers an “Enter” keystroke on the newly focused dialog. This triggers the default “Save” action.
- Sandbox Escape / Unauthorized Access: The dialog closes, and the browser process grants the compromised renderer a valid
FileSystemAccessFileHandleformalware.bat, allowing the attacker to silently write arbitrary payloads to the user’s disk. This identical pattern can be used to silently grant access to connected USB/Bluetooth/Serial hardware.
Recommendation
All sensitive Mojo methods that trigger browser-side choosers or pickers must strictly verify that transient user activation is present in the calling frame.
- For
ChooseEntries, the return value ofUpdateUserActivationState(kConsumeTransientActivation)must be checked, and the request should be rejected (e.g., returningkPermissionDenied) if it isfalse. - For the hardware and sharing APIs, a similar browser-side check to consume transient user activation must be implemented before calling
RunChooseror equivalent.
Evaluated with Chrome root at commit: 9760e6c70cd33a320713361f17c6dcca85648c0f
Results from Flapjack so far have been promising, but it can be wrong in its deductions. At this time, it does not produce proof of concepts or fuzzer tests. If this proves to be a false positive, please close as WAI; data from false positives will be used to improve Flapjack’s accuracy over time. And please feel free to reach out to me directly if you have concerns or feedback on the project.