Chrome · Actor
CVE-2026-13930
Logic Error in Actor
Overview
Medium
Severity
—
CVSS
No
Exploited ITW
Fixed
Fix Status
Changed Functions
| Function | Change | Notes |
|---|---|---|
target_state_chrome/browser/actor/actor_test_util.cc |
modified | |
GetActionSequenceDelegatechrome/browser/actor/execution_engine.cc |
modified |
Files Changed
chrome/browser/actor/actor_test_util.ccchrome/browser/actor/execution_engine.ccchrome/browser/actor/execution_engine.h
Patch
From 5385eff09512393e4cadb34775b5388f174c6dae Mon Sep 17 00:00:00 2001 From: Chris Fredrickson <[email protected]> Date: Tue, 19 May 2026 09:43:11 -0700 Subject: [PATCH] [glic] Use longer-lived WeakPtr in ActorNavigationThrottle First, this CL renames ExecutionEngine::GetWeakPtr to EE::GetActionSequenceWeakPtr, since the pointers are invalidated when the action sequence is finished (as opposed to normal WeakPtr usage). Then, it adds a GetWeakPtr member with the usual semantics, for use in ActorNavigationThrottle. This fixes a bug where slow navigations would cease to follow safety checks if their associated action sequence timed out (even if the task was still active). Fixed: 512937764 Change-Id: I61262231ed5105112d74f186824d2b10eae2a0ae Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7858075 Auto-Submit: Chris Fredrickson <[email protected]> Reviewed-by: Kevin McNee <[email protected]> Commit-Queue: Chris Fredrickson <[email protected]> Commit-Queue: Kevin McNee <[email protected]> Cr-Commit-Position: refs/heads/main@{#1632939} --- diff --git a/chrome/browser/actor/actor_test_util.cc b/chrome/browser/actor/actor_test_util.cc index feda9a8..178ae0a 100644 --- a/chrome/browser/actor/actor_test_util.cc +++ b/chrome/browser/actor/actor_test_util.cc @@ -793,7 +793,7 @@ ExecutionEngine& execution_engine, ExecutionEngine::State target_state) : callback_(std::move(callback)), - execution_engine_(execution_engine.GetWeakPtr()), + execution_engine_(execution_engine.GetActionSequenceWeakPtr()), target_state_(target_state) { execution_engine_->AddObserver(this); } diff --git a/chrome/browser/actor/execution_engine.cc b/chrome/browser/actor/execution_engine.cc index db8103b..aa42bb7 100644 --- a/chrome/browser/actor/execution_engine.cc +++ b/chrome/browser/actor/execution_engine.cc @@ -338,14 +338,15 @@ base::SequencedTaskRunner::GetCurrentDefault()->PostTask( FROM_HERE, base::BindOnce( - &ExecutionEngine::CheckNavigationSensitiveUrlList, GetWeakPtr(), - source_origin, navigation_handle.GetInitiatorOrigin(), + &ExecutionEngine::CheckNavigationSensitiveUrlList, + GetActionSequenceWeakPtr(), source_origin, + navigation_handle.GetInitiatorOrigin(), navigation_handle.GetURL(), GetPrimaryMainFrame(navigation_handle)->GetPageUkmSourceId(), skip_prompt, std::move(timer), std::move(callback).Then(base::BindOnce( &ExecutionEngine::MaybeRecordNavigationConfirmationMetrics, - GetWeakPtr(), state(), + GetActionSequenceWeakPtr(), state(), url::Origin::Create(navigation_handle.GetURL()), /*is_pre_approved=*/false)))); return content::NavigationThrottle::DEFER; @@ -431,9 +432,9 @@ MaybeCheckOptimizationGuideForSensitiveUrl( destination_url, task_->GetProfile(), base::BindOnce(&ExecutionEngine::OnNavigationSensitiveUrlListChecked, - GetWeakPtr(), source, initiator, destination_origin, - ukm_source_id, skip_prompt, std::move(timer), - std::move(callback))); + GetActionSequenceWeakPtr(), source, initiator, + destination_origin, ukm_source_id, skip_prompt, + std::move(timer), std::move(callback))); if (!sensitive_check_result.has_value()) { std::move(sensitive_check_result).error().Run(/*not_sensitive=*/true); } @@ -528,8 +529,8 @@ SendNavigationConfirmationRequest( destination, base::BindOnce(&ExecutionEngine::OnNavigationConfirmationDecision, - GetWeakPtr(), destination, ukm_source_id, std::move(timer), - state_, std::move(callback))); + GetActionSequenceWeakPtr(), destination, ukm_source_id, + std::move(timer), state_, std::move(callback))); } void ExecutionEngine::SendNavigationConfirmationRequest( @@ -642,7 +643,8 @@ task_->delegate()->RequestToShowUserConfirmationDialog( task_->id(), destination, for_sensitive_origin, base::BindOnce(&ExecutionEngine::OnPromptUserToConfirmNavigationDecision, - GetWeakPtr(), destination, std::move(callback))); + GetActionSequenceWeakPtr(), destination, + std::move(callback))); } void ExecutionEngine::OnPromptUserToConfirmNavigationDecision( @@ -715,6 +717,10 @@ GetInProgressAction().RequiresOpeningWebContents(); } +base::WeakPtr<ExecutionEngine> ExecutionEngine::GetWeakPtr() { + return weak_ptr_factory_.GetWeakPtr(); +} + void ExecutionEngine::CancelOngoingActions(mojom::ActionResultCode reason) { TRACE_EVENT0("actor", "ExecutionEngine::CancelOngoingActions"); deferred_finish_tool_invoke_.Reset(); @@ -889,7 +895,7 @@ MayActOnTab( *tab, *journal_, task_->id(), origin_checker_, task_->policy_checker(), base::BindOnce( - &ExecutionEngine::OnMayActOnTabDecision, GetWeakPtr(), + &ExecutionEngine::OnMayActOnTabDecision, GetActionSequenceWeakPtr(), tab->GetContents()->GetPrimaryMainFrame()->GetLastCommittedOrigin())); } @@ -913,7 +919,8 @@ /*timer=*/std::nullopt, std::move(response_to_result_code) .Then(base::BindOnce(&ExecutionEngine::DidFinishAsyncSafetyChecks, - GetWeakPtr(), evaluated_origin))); + GetActionSequenceWeakPtr(), + evaluated_origin))); return; } @@ -997,8 +1004,8 @@ SetState(State::kToolCreateAndVerify); tool_controller_->CreateToolAndValidate( - GetInProgressAction(), - base::BindOnce(&ExecutionEngine::PostToolCreate, GetWeakPtr())); + GetInProgressAction(), base::BindOnce(&ExecutionEngine::PostToolCreate, + GetActionSequenceWeakPtr())); } void ExecutionEngine::PostToolCreate(mojom::ActionResultPtr result) { @@ -1010,7 +1017,8 @@ SetState(State::kUiPreInvoke); ui_event_dispatcher_->OnPreTool( GetInProgressAction(), - base::BindOnce(&ExecutionEngine::FinishedUiPreInvoke, GetWeakPtr())); + base::BindOnce(&ExecutionEngine::FinishedUiPreInvoke, + GetActionSequenceWeakPtr())); } void ExecutionEngine::FinishedUiPreInvoke(mojom::ActionResultPtr result) { @@ -1022,8 +1030,8 @@ } SetState(State::kToolInvoke); - tool_controller_->Invoke( - base::BindOnce(&ExecutionEngine::FinishedToolInvoke, GetWeakPtr())); + tool_controller_->Invoke(base::BindOnce(&ExecutionEngine::FinishedToolInvoke, + GetActionSequenceWeakPtr())); } void ExecutionEngine::FinishedToolInvoke(mojom::ActionResultPtr result) { @@ -1085,7 +1093,8 @@ SetState(State::kUiPostInvoke); ui_event_dispatcher_->OnPostTool( GetInProgressAction(), - base::BindOnce(&ExecutionEngine::FinishedUiPostInvoke, GetWeakPtr())); + base::BindOnce(&ExecutionEngine::FinishedUiPostInvoke, + GetActionSequenceWeakPtr())); } void ExecutionEngine::FinishedUiPostInvoke(mojom::ActionResultPtr result) { @@ -1160,7 +1169,7 @@ actions_weak_ptr_factory_.InvalidateWeakPtrs(); } -base::WeakPtr<ExecutionEngine> ExecutionEngine::GetWeakPtr() { +base::WeakPtr<ExecutionEngine> ExecutionEngine::GetActionSequenceWeakPtr() { return actions_weak_ptr_factory_.GetWeakPtr(); } @@ -1239,8 +1248,9 @@ affiliation_service->GetAffiliationsAndBranding( affiliations::FacetURI::FromPotentiallyInvalidSpec( origin.GetURL().GetWithEmptyPath().spec()), - base::BindOnce(&ExecutionEngine::OnAffiliationsReceived, GetWeakPtr(), - origin, std::move(affiliations_fetched))); + base::BindOnce(&ExecutionEngine::OnAffiliationsReceived, + GetActionSequenceWeakPtr(), origin, + std::move(affiliations_fetched))); } else { std::move(affiliations_fetched).Run(); } @@ -1350,7 +1360,7 @@ base::WeakPtr<actor_login::ActionSequenceDelegate> ExecutionEngine::GetActionSequenceDelegate() { - return actions_weak_ptr_factory_.GetWeakPtr(); + return GetActionSequenceWeakPtr(); } base::CallbackListSubscription ExecutionEngine::RegisterActionSequenceEnded( diff --git a/chrome/browser/actor/execution_engine.h b/chrome/browser/actor/execution_engine.h index 0925186..4545c57 100644 --- a/chrome/browser/actor/execution_engine.h
Loading diff…
Regression Test / PoC
shipped with the fix
diff --git a/chrome/browser/actor/execution_engine_origin_gating_browsertest.cc b/chrome/browser/actor/execution_engine_origin_gating_browsertest.cc
index 6ae86ee1..b80b18f 100644
--- a/chrome/browser/actor/execution_engine_origin_gating_browsertest.cc
+++ b/chrome/browser/actor/execution_engine_origin_gating_browsertest.cc
@@ -32,6 +32,7 @@
#include "content/public/test/content_browser_test_utils.h"
#include "content/public/test/test_navigation_observer.h"
#include "net/dns/mock_host_resolver.h"
+#include "net/test/embedded_test_server/controllable_http_response.h"
#include "services/metrics/public/cpp/ukm_builders.h"
#include "url/origin.h"
#include "url/url_util.h"
@@ -2381,4 +2382,83 @@
: "ConfirmOriginsDisabled";
});
+class ExecutionEngineOriginGatingSlowResponseBrowserTest
+ : public ExecutionEngineOriginGatingBrowserTestBase {
+ public:
+ ExecutionEngineOriginGatingSlowResponseBrowserTest() {
+ scoped_feature_list_.InitWithFeaturesAndParameters(
+ /*enabled_features=*/
+ {
+ {features::kGlicActor,
+ {{features::kGlicActorPolicyControlExemption.name, "true"},
+ {features::kGlicActorPageStabilityTimeout.name, "300ms"},
+ {features::kActorObservationDelayTimeout.name, "1s"}}},
+ },
+ /*disabled_features=*/{});
+ }
+
+ void SetUpOnMainThread() override {
+ CHECK(!embedded_https_test_server().Started());
+ response_manager_ =
+ std::make_unique<net::test_server::ControllableHttpResponseManager>(
+ &embedded_https_test_server(), "/slow");
+ ExecutionEngineOriginGatingBrowserTestBase::SetUpOnMainThread();
+ }
+
+ protected:
+ std::unique_ptr<net::test_server::ControllableHttpResponseManager>
+ response_manager_;
+
+ private:
+ base::test::ScopedFeatureList scoped_feature_list_;
+};
+
+// Verifies that navigations are subject to safety checks even if the relevant
+// action sequence times out (and is therefore cancelled).
+IN_PROC_BROWSER_TEST_F(ExecutionEngineOriginGatingSlowResponseBrowserTest,
+ SlowResponseDoesntBypassNavGating) {
+ base::HistogramTester histogram_tester;
+
+ const GURL start_url =
+ embedded_https_test_server().GetURL("example.com", "/actor/link.html");
+
+ ASSERT_TRUE(content::NavigateToURL(web_contents(), start_url));
+ OpenGlicAndCreateTask();
+
+ ASSERT_TRUE(content::ExecJs(
+ web_contents(),
+ content::JsReplace("setLink($1);", embedded_https_test_server().GetURL(
+ "blocked.example.com", "/slow"))));
+
+ ActResultFuture act_result;
+ content::TestNavigationObserver nav_observer(web_contents());
+ actor_task().Act(ToRequestList(MakeClickRequest(
+ *main_frame(),
+ content::GetDOMNodeId(*main_frame(), "#link").value())),
+ act_result.GetCallback());
+ // No handler has been registered; wait for the observation delay to cancel
+ // the action.
+ ASSERT_TRUE(act_result.Wait());
+
+ std::unique_ptr<net::test_server::ControllableHttpResponse> slow_response =
+ response_manager_->WaitForRequest();
+ slow_response->Send(net::HTTP_OK);
+ slow_response->Done();
+ nav_observer.Wait();
+
+ EXPECT_FALSE(nav_observer.last_navigation_succeeded());
+ EXPECT_EQ(web_contents()->GetLastCommittedURL(), start_url);
+
+ histogram_tester.ExpectUniqueSample(
+ "Actor.NavigationGating.GatingDecision2",
+ /*sample=*/ExecutionEngine::GatingDecision::kNeedsAsyncCheck,
+ /*expected_bucket_count=*/1);
+ histogram_tester.ExpectUniqueSample("Actor.NavigationGating.AppliedGate",
+ /*sample=*/true,
+ /*expected_bucket_count=*/1);
+ histogram_tester.ExpectUniqueSample(
+ "Actor.NavigationGating.PermissionGranted", /*sample=*/false,
+ /*expected_bucket_count=*/1);
+}
+
} // namespace actor
Loading diff…
Original Bug Report
reported by [email protected]
Potential security prompt bypass via WeakPtr fail-open in ActorNavigationThrottle
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 without the Chrome Security team. Please see https://chromium.googlesource.com/chromium/src/+/main/docs/security/ai-generated-security-bugs-faq.md for more information.
Overview: A potential logic flaw in ActorNavigationThrottle can allow navigations to bypass Glic security gates. The throttle fails open if its WeakPtr to the ExecutionEngine is invalidated, which occurs prematurely when an action sequence completes. An attacker can exploit this by delaying a navigation response until the action sequence finishes, allowing a redirect to a sensitive origin without triggering user confirmation prompts.
Affected files:
chrome/browser/actor/actor_navigation_throttle.ccchrome/browser/actor/execution_engine.ccchrome/browser/actor/site_policy.cc
Estimated timestamp from git blame: 2025-08-20
Summary
A potential logic flaw in ActorNavigationThrottle allows navigations to bypass crucial security checks, including the Optimization Guide blocklist and user confirmation prompts for novel origins. The vulnerability stems from a lifetime mismatch: the throttle relies on a WeakPtr that is scoped to an individual action sequence rather than the ExecutionEngine itself. If the action sequence finishes before the navigation response is processed, the throttle fails open and allows the navigation to proceed unconditionally.
Root Cause
When an ActorNavigationThrottle is created, it obtains a WeakPtr to the ExecutionEngine:
// chrome/browser/actor/actor_navigation_throttle.cc
execution_engine_(task.GetExecutionEngine().GetWeakPtr())
However, ExecutionEngine::GetWeakPtr() does not return a pointer tied to the engine’s lifetime. Instead, it returns a pointer tied to actions_weak_ptr_factory_, which is invalidated at the end of every action sequence:
// chrome/browser/actor/execution_engine.cc
void ExecutionEngine::CompleteActions(...) {
// ...
actions_weak_ptr_factory_.InvalidateWeakPtrs();
}
When the navigation response finally arrives, ActorNavigationThrottle::WillProcessResponse() checks this pointer. If the action sequence has completed, the pointer is null, and the throttle returns PROCEED:
// chrome/browser/actor/actor_navigation_throttle.cc
if (!execution_engine_) {
return content::NavigationThrottle::PROCEED; // Fail-open bypass
}
This bypasses execution_engine_->ShouldDeferNavigation(), which is responsible for enforcing the navigation gating policy (e.g., checking if the origin is confirmed by the user or checking the Optimization Guide list). Notably, the initial request check in MayActOnUrl short-circuits and assumes ShouldDeferNavigation will handle the actual origin gating.
Potential Attack Scenario
Note: These are suggested steps; our tooling agent does not have the capability to run a live proof of concept.
- The user is on an attacker-controlled page while a Glic Actor task is active.
- The attacker-controlled page initiates a navigation to an attacker-controlled endpoint (e.g.,
https://attacker.example/delay). - The attacker’s server intentionally stalls the HTTP response for several seconds.
- The
ActorNavigationThrottleis created.WillStartRequest()callsMayActOnUrl(), which immediately allows the request, assumingShouldDeferNavigation()will catch issues later. - While the response is delayed, the active Glic action sequence finishes (e.g., an observation timeout occurs).
ExecutionEngine::CompleteActions()is called, invalidatingactions_weak_ptr_factory_. - The attacker’s server finally responds with an HTTP 302 redirect to a sensitive, blocked, or novel origin.
- When
WillProcessResponse()is called for the new URL,execution_engine_evaluates to null. - The throttle returns
content::NavigationThrottle::PROCEED. The navigation commits to the sensitive origin without triggering any user confirmation prompts or security checks.
Suggested Fix
There are two primary ways to address this:
- Use a separate WeakPtrFactory: The
ActorNavigationThrottleneeds to track the lifetime of theExecutionEngineobject, not the current action sequence. Add a standardbase::WeakPtrFactory<ExecutionEngine> weak_ptr_factory_{this};toExecutionEnginethat is not invalidated inCompleteActions(), and use that for the throttle. - Fail Closed: If the actor task is actively tracking a tab, but the
ExecutionEngineunexpectedly goes away or loses its sequence context during an in-flight navigation,WillProcessResponse()should returncontent::NavigationThrottle::CANCEL_AND_IGNORErather thanPROCEEDif it’s supposed to be enforcing strict gating.
Evaluated with Chrome root at commit: b3153093eb3c78c3e88ccf562bcbc20437a04b0e
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.
References
On This Page