From 1fc7b5f1b300973956c4edcb7e3f8f8d56a56e6a Mon Sep 17 00:00:00 2001
From: Nidhi Jaju <nidhijaju@chromium.org>
Date: Mon, 06 Jul 2026 03:52:17 -0700
Subject: [PATCH] Validate browser-initiated file uploads

This CL introduces token-based tracking and validation for files
uploaded by the browser process to the network service.

We introduce `BrowserFileAccessCallbacks` to `SimpleURLLoader`, allowing
the browser to register file paths with `ChildProcessSecurityPolicy`.
`ScopedBrowserFileAccess` manages token lifetimes for file paths
originating from navigations (e.g., Web Share Target).

Finally, `NetworkContextClientBaseImpl::HandleFileUploadRequest` is
updated to validate the tokens using `CanBrowserReadFile()` when the
originating process is the browser. This entire mechanism is gated
behind the `kFileUploadTokenRegistration` feature flag, which is
enabled by default.

Bug: 497428001
Change-Id: I8a06849dc69b19d3fa71c62ba2ed63813db686fa
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7968017
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Auto-Submit: Nidhi Jaju <nidhijaju@chromium.org>
Commit-Queue: Nidhi Jaju <nidhijaju@chromium.org>
Reviewed-by: Takashi Toyoshima <toyoshim@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1657089}
---

diff --git a/content/browser/BUILD.gn b/content/browser/BUILD.gn
index 6be9945..700ebede 100644
--- a/content/browser/BUILD.gn
+++ b/content/browser/BUILD.gn
@@ -2172,6 +2172,8 @@
     "scheduler/responsiveness/watcher.h",
     "scoped_active_url.cc",
     "scoped_active_url.h",
+    "scoped_browser_file_access.cc",
+    "scoped_browser_file_access.h",
     "screen_details/screen_change_monitor.cc",
     "screen_details/screen_change_monitor.h",
     "screen_orientation/screen_orientation_provider.cc",
diff --git a/content/browser/browser_main_loop.cc b/content/browser/browser_main_loop.cc
index 865c302..bc44a5da 100644
--- a/content/browser/browser_main_loop.cc
+++ b/content/browser/browser_main_loop.cc
@@ -112,6 +112,7 @@
 #include "content/public/browser/browser_main_parts.h"
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
+#include "content/public/browser/child_process_security_policy.h"
 #include "content/public/browser/content_browser_client.h"
 #include "content/public/browser/device_service.h"
 #include "content/public/browser/network_service_instance.h"
@@ -147,7 +148,9 @@
 #include "services/audio/service.h"
 #include "services/data_decoder/public/cpp/service_provider.h"
 #include "services/data_decoder/public/mojom/data_decoder_service.mojom.h"
+#include "services/network/public/cpp/features.h"
 #include "services/network/public/cpp/network_switches.h"
+#include "services/network/public/cpp/simple_url_loader.h"
 #include "services/network/public/mojom/network_service.mojom.h"
 #include "services/network/transitional_url_loader_factory_owner.h"
 #include "services/tracing/public/cpp/background_tracing/background_tracing_manager.h"
@@ -830,6 +833,24 @@
     RenderProcessHost::SetRunRendererInProcess(true);
 #endif
 
+  // Set up the callbacks used by the network layer to track and validate
+  // file access for browser-initiated uploads.
+  if (base::FeatureList::IsEnabled(
+          network::features::kBrowserInitiatedFileUploadValidation)) {
+    network::SimpleURLLoader::FileUploadEventCallbacks callbacks;
+    callbacks.register_callback = base::BindRepeating(
+        [](const base::UnguessableToken& token, const base::FilePath& path) {
+          ChildProcessSecurityPolicy::GetInstance()->GrantFileForBrowserUpload(
+              token, path);
+        });
+    callbacks.revoke_callback =
+        base::BindRepeating([](const base::UnguessableToken& token) {
+          ChildProcessSecurityPolicy::GetInstance()->RevokeFileForBrowserUpload(
+              token);
+        });
+    network::SimpleURLLoader::SetFileUploadEventCallbacks(callbacks);
+  }
+
   // Initialize origins that require process isolation.  Must be done
   // after base::FeatureList is initialized, but before any navigations can
   // happen.
diff --git a/content/browser/loader/navigation_url_loader_impl.cc b/content/browser/loader/navigation_url_loader_impl.cc
index d004578..c5073932 100644
--- a/content/browser/loader/navigation_url_loader_impl.cc
+++ b/content/browser/loader/navigation_url_loader_impl.cc
@@ -2170,6 +2170,17 @@
       browser_context_, storage_partition_, frame_tree_node,
       ukm::SourceIdObj::FromInt64(ukm_source_id_), &bypass_redirect_checks_,
       allow_same_site_none_cookies_override_);
+
+  if (base::FeatureList::IsEnabled(
+          network::features::kBrowserInitiatedFileUploadValidation) &&
+      resource_request_->request_body) {
+    std::vector<base::FilePath> files =
+        resource_request_->request_body->GetReferencedFiles();
+    if (!files.empty()) {
+      scoped_browser_file_access_ =
+          std::make_unique<ScopedBrowserFileAccess>(std::move(files));
+    }
+  }
 }
 
 // static
diff --git a/content/browser/loader/navigation_url_loader_impl.h b/content/browser/loader/navigation_url_loader_impl.h
index 8dcbce48..9dbfdec 100644
--- a/content/browser/loader/navigation_url_loader_impl.h
+++ b/content/browser/loader/navigation_url_loader_impl.h
@@ -14,6 +14,7 @@
 #include "content/browser/loader/navigation_url_loader.h"
 #include "content/browser/loader/response_head_update_params.h"
 #include "content/browser/navigation_subresource_loader_params.h"
+#include "content/browser/scoped_browser_file_access.h"
 #include "content/common/content_export.h"
 #include "content/public/browser/frame_tree_node_id.h"
 #include "content/public/browser/global_request_id.h"
@@ -599,6 +600,12 @@
   // response.
   ResponseHeadUpdateParams head_update_params_;
 
+  // If the navigation request includes a file upload, this object ensures the
+  // browser process is aware that the Network Service is allowed to read the
+  // files. The files are granted access upon creation and revoked when this
+  // loader is destroyed.
+  std::unique_ptr<ScopedBrowserFileAccess> scoped_browser_file_access_;
+
   base::WeakPtrFactory<NavigationURLLoaderImpl> weak_factory_{this};
 };
 
diff --git a/content/browser/network_context_client_base_impl.cc b/content/browser/network_context_client_base_impl.cc
index ecc0497c..b4c5a2b4 100644
--- a/content/browser/network_context_client_base_impl.cc
+++ b/content/browser/network_context_client_base_impl.cc
@@ -19,6 +19,7 @@
 #include "content/public/common/content_client.h"
 #include "mojo/public/cpp/bindings/remote.h"
 #include "net/base/net_errors.h"
+#include "services/network/public/cpp/features.h"
 #include "services/network/public/mojom/trust_tokens.mojom.h"
 
 namespace content {
@@ -38,9 +39,17 @@
                         (async ? base::File::FLAG_ASYNC : 0);
   ChildProcessSecurityPolicy* cpsp = ChildProcessSecurityPolicy::GetInstance();
   for (const auto& file_path : file_paths) {
-    if (!process_id.is_browser() &&
-        !cpsp->CanReadFile(ToChildProcessId(process_id.renderer_process_id()),
-                           file_path)) {
+    bool access_denied = false;
+    if (base::FeatureList::IsEnabled(
+            network::features::kBrowserInitiatedFileUploadValidation) &&
+        process_id.is_browser()) {
+      access_denied = !cpsp->CanReadFileForBrowserUpload(file_path);
+    } else if (!process_id.is_browser()) {
+      access_denied = !cpsp->CanReadFile(
+          ToChildProcessId(process_id.renderer_process_id()), file_path);
+    }
+
+    if (access_denied) {
       task_runner->PostTask(
           FROM_HERE, base::BindOnce(std::move(callback), net::ERR_ACCESS_DENIED,
                                     std::vector<base::File>()));
diff --git a/content/browser/network_context_client_base_impl_unittest.cc b/content/browser/network_context_client_base_impl_unittest.cc
index 2cbf0e6..7da23ceb 100644
--- a/content/browser/network_context_client_base_impl_unittest.cc
+++ b/content/browser/network_context_client_base_impl_unittest.cc
@@ -14,7 +14,9 @@
 #include "base/strings/string_view_util.h"
 #include "base/test/task_environment.h"
 #include "base/test/test_file_util.h"
+#include "base/test/test_future.h"
 #include "base/types/fixed_array.h"
+#include "base/unguessable_token.h"
 #include "build/build_config.h"
 #include "content/browser/security/cpsp/child_process_security_policy_impl.h"
 #include "content/public/browser/network_context_client_base.h"
@@ -223,19 +225,39 @@
   EXPECT_EQ(0U, response.opened_files.size());
 }
 
-TEST_F(NetworkContextClientBaseTest, UploadFromBrowserProcess) {
+TEST_F(NetworkContextClientBaseTest,
+       OnFileUploadRequested_BrowserProcess_AccessDenied) {
   base::FilePath path = temp_dir_.GetPath().AppendASCII("filename");
   CreateFile(path, kFileContent1);
-  // No grant necessary for browser process.
 
-  UploadResponse response;
+  base::test::TestFuture<int, std::vector<base::File>> future;
   client_.OnFileUploadRequested(
       network::OriginatingProcessId::browser(), false, {path},
-      /*destination_url=*/GURL(), std::move(response.callback));
-  task_environment_.RunUntilIdle();

diff --git a/content/browser/network_context_client_base_impl_unittest.cc b/content/browser/network_context_client_base_impl_unittest.cc
index 2cbf0e6..7da23ceb 100644
--- a/content/browser/network_context_client_base_impl_unittest.cc
+++ b/content/browser/network_context_client_base_impl_unittest.cc
@@ -14,7 +14,9 @@
 #include "base/strings/string_view_util.h"
 #include "base/test/task_environment.h"
 #include "base/test/test_file_util.h"
+#include "base/test/test_future.h"
 #include "base/types/fixed_array.h"
+#include "base/unguessable_token.h"
 #include "build/build_config.h"
 #include "content/browser/security/cpsp/child_process_security_policy_impl.h"
 #include "content/public/browser/network_context_client_base.h"
@@ -223,19 +225,39 @@
   EXPECT_EQ(0U, response.opened_files.size());
 }
 
-TEST_F(NetworkContextClientBaseTest, UploadFromBrowserProcess) {
+TEST_F(NetworkContextClientBaseTest,
+       OnFileUploadRequested_BrowserProcess_AccessDenied) {
   base::FilePath path = temp_dir_.GetPath().AppendASCII("filename");
   CreateFile(path, kFileContent1);
-  // No grant necessary for browser process.
 
-  UploadResponse response;
+  base::test::TestFuture<int, std::vector<base::File>> future;
   client_.OnFileUploadRequested(
       network::OriginatingProcessId::browser(), false, {path},
-      /*destination_url=*/GURL(), std::move(response.callback));
-  task_environment_.RunUntilIdle();
-  EXPECT_EQ(net::OK, response.error_code);
-  ASSERT_EQ(1U, response.opened_files.size());
-  ValidateFileContents(response.opened_files[0], kFileContent1);
+      /*destination_url=*/GURL(), future.GetCallback());
+  EXPECT_EQ(net::ERR_ACCESS_DENIED, future.Get<0>());
+  EXPECT_EQ(0U, future.Get<1>().size());
+}
+
+TEST_F(NetworkContextClientBaseTest,
+       OnFileUploadRequested_BrowserProcess_AccessGranted) {
+  base::FilePath path = temp_dir_.GetPath().AppendASCII("filename");
+  CreateFile(path, kFileContent1);
+
+  base::UnguessableToken token = base::UnguessableToken::Create();
+  ChildProcessSecurityPolicyImpl::GetInstance()->GrantFileForBrowserUpload(
+      token, path);
+
+  base::test::TestFuture<int, std::vector<base::File>> future;
+  client_.OnFileUploadRequested(
+      network::OriginatingProcessId::browser(), false, {path},
+      /*destination_url=*/GURL(), future.GetCallback());
+  EXPECT_EQ(net::OK, future.Get<0>());
+  std::vector<base::File> opened_files = std::get<1>(future.Take());
+  ASSERT_EQ(1U, opened_files.size());
+  ValidateFileContents(opened_files[0], kFileContent1);
+
+  ChildProcessSecurityPolicyImpl::GetInstance()->RevokeFileForBrowserUpload(
+      token);
 }
 
 }  // namespace content
diff --git a/content/browser/security/cpsp/child_process_security_policy_unittest.cc b/content/browser/security/cpsp/child_process_security_policy_unittest.cc
index 9f442fed..11cec88 100644
--- a/content/browser/security/cpsp/child_process_security_policy_unittest.cc
+++ b/content/browser/security/cpsp/child_process_security_policy_unittest.cc
@@ -3595,6 +3595,101 @@
   p->EraseOriginAgentClusterState(browsing_instance_id);
 }
 
+TEST_P(ChildProcessSecurityPolicyTest,
+       BrowserFileAccess_GrantAndRevokeSingleToken) {
+  auto* p = ChildProcessSecurityPolicyImpl::GetInstance();
+  base::FilePath file(TEST_PATH("/foo/bar"));
+  base::UnguessableToken token = base::UnguessableToken::Create();
+
+  EXPECT_FALSE(p->CanReadFileForBrowserUpload(file));
+
+  p->GrantFileForBrowserUpload(token, file);
+  EXPECT_TRUE(p->CanReadFileForBrowserUpload(file));
+
+  p->RevokeFileForBrowserUpload(token);
+  EXPECT_FALSE(p->CanReadFileForBrowserUpload(file));
+}
+
+TEST_P(ChildProcessSecurityPolicyTest,
+       BrowserFileAccess_MultipleTokensSameFile) {
+  auto* p = ChildProcessSecurityPolicyImpl::GetInstance();
+  base::FilePath file(TEST_PATH("/foo/bar"));
+  base::UnguessableToken token1 = base::UnguessableToken::Create();
+  base::UnguessableToken token2 = base::UnguessableToken::Create();
+
+  EXPECT_FALSE(p->CanReadFileForBrowserUpload(file));
+
+  p->GrantFileForBrowserUpload(token1, file);
+  p->GrantFileForBrowserUpload(token2, file);
+  EXPECT_TRUE(p->CanReadFileForBrowserUpload(file));
+
+  p->RevokeFileForBrowserUpload(token1);
+  EXPECT_TRUE(p->CanReadFileForBrowserUpload(file));
+
+  p->RevokeFileForBrowserUpload(token2);
+  EXPECT_FALSE(p->CanReadFileForBrowserUpload(file));
+}
+
+TEST_P(ChildProcessSecurityPolicyTest,
+       BrowserFileAccess_MultipleTokensDifferentFiles) {
+  auto* p = ChildProcessSecurityPolicyImpl::GetInstance();
+  base::FilePath file1(TEST_PATH("/foo/bar1"));
+  base::FilePath file2(TEST_PATH("/foo/bar2"));
+  base::UnguessableToken token1 = base::UnguessableToken::Create();
+  base::UnguessableToken token2 = base::UnguessableToken::Create();
+
+  p->GrantFileForBrowserUpload(token1, file1);
+  p->GrantFileForBrowserUpload(token2, file2);
+
+  EXPECT_TRUE(p->CanReadFileForBrowserUpload(file1));
+  EXPECT_TRUE(p->CanReadFileForBrowserUpload(file2));
+
+  p->RevokeFileForBrowserUpload(token1);
+  EXPECT_FALSE(p->CanReadFileForBrowserUpload(file1));
+  EXPECT_TRUE(p->CanReadFileForBrowserUpload(file2));
+
+  p->RevokeFileForBrowserUpload(token2);
+  EXPECT_FALSE(p->CanReadFileForBrowserUpload(file1));
+  EXPECT_FALSE(p->CanReadFileForBrowserUpload(file2));
+}
+
+TEST_P(ChildProcessSecurityPolicyTest,
+       BrowserFileAccess_OneTokenMultipleFiles) {
+  auto* p = ChildProcessSecurityPolicyImpl::GetInstance();
+  base::FilePath file1(TEST_PATH("/foo/bar1"));
+  base::FilePath file2(TEST_PATH("/foo/bar2"));
+  base::UnguessableToken token = base::UnguessableToken::Create();
+
+  p->GrantFileForBrowserUpload(token, file1);
+  p->GrantFileForBrowserUpload(token, file2);
+
+  EXPECT_TRUE(p->CanReadFileForBrowserUpload(file1));
+  EXPECT_TRUE(p->CanReadFileForBrowserUpload(file2));
+
+  p->RevokeFileForBrowserUpload(token);
+  EXPECT_FALSE(p->CanReadFileForBrowserUpload(file1));
+  EXPECT_FALSE(p->CanReadFileForBrowserUpload(file2));
+}
+
+TEST_P(ChildProcessSecurityPolicyTest,
+       BrowserFileAccess_RevokeNonExistentToken) {
+  auto* p = ChildProcessSecurityPolicyImpl::GetInstance();
+  base::FilePath file(TEST_PATH("/foo/bar"));
+  base::UnguessableToken valid_token = base::UnguessableToken::Create();
+  base::UnguessableToken invalid_token = base::UnguessableToken::Create();
+
+  p->GrantFileForBrowserUpload(valid_token, file);
+  EXPECT_TRUE(p->CanReadFileForBrowserUpload(file));
+
+  // Revoking a token that was never granted access shouldn't crash or modify
+  // other grants.
+  p->RevokeFileForBrowserUpload(invalid_token);
+  EXPECT_TRUE(p->CanReadFileForBrowserUpload(file));
+
+  p->RevokeFileForBrowserUpload(valid_token);
+  EXPECT_FALSE(p->CanReadFileForBrowserUpload(file));
+}
+
 // This intentionally excludes {kCppOnly, kProcessState} since that behaves
 // the same as {kCppOnly, kMain} and is redundant to test separately.
 const CpspTestParam kCpspTestParams[] = {