From 8f31946dc5d9a59877c589da2d4b03c6f8739bf3 Mon Sep 17 00:00:00 2001
From: Alex Moshchuk <[email protected]>
Date: Tue, 07 Apr 2026 09:33:15 -0700
Subject: [PATCH] Guard against unwanted path traversal with dot-dot-space in CPSP.

Previously, ChildProcessSecurityPolicyImpl::HasPermissionsForFile()
handled parent references in the FilePath by comparing directly to
base::FilePath::kParentDirectory. In some obscure corner cases on
Windows, this is insufficient, as strings like ".. " (dot-dot-space)
could fail to be recognized as parent dir references, allowing a
renderer process to potentially access files outside of the path it
was previously granted access for. Fix this by using
FilePath::ReferencesParent() instead, which already properly deals
with these corner cases.

Note that it appears that the ".. " trick only works when it's at the
end of a path, and not in the middle. However, this CL still adds
tests for both cases (both written with help from Gemini).

Change-Id: I2897a271cb82cac7e19c1afd802d6be52fe11f02
Bug: 498887785
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7728123
Commit-Queue: Alex Moshchuk <[email protected]>
Reviewed-by: Charlie Reis <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1610818}
---

diff --git a/content/browser/child_process_security_policy_impl.cc b/content/browser/child_process_security_policy_impl.cc
index a033e10..0f535e5 100644
--- a/content/browser/child_process_security_policy_impl.cc
+++ b/content/browser/child_process_security_policy_impl.cc
@@ -798,7 +798,7 @@
     int skip = 0;
     while (current_path != last_path) {
       base::FilePath base_name = current_path.BaseName();
-      if (base_name.value() == base::FilePath::kParentDirectory) {
+      if (base_name.ReferencesParent()) {
         ++skip;
       } else if (skip > 0) {
         if (base_name.value() != base::FilePath::kCurrentDirectory) {
diff --git a/content/browser/renderer_host/file_utilities_host_impl.h b/content/browser/renderer_host/file_utilities_host_impl.h
index 5ae1a8d0..c68b9cc6 100644
--- a/content/browser/renderer_host/file_utilities_host_impl.h
+++ b/content/browser/renderer_host/file_utilities_host_impl.h
@@ -6,13 +6,15 @@
 #define CONTENT_BROWSER_RENDERER_HOST_FILE_UTILITIES_HOST_IMPL_H_
 
 #include "build/build_config.h"
+#include "content/common/content_export.h"
 #include "content/public/common/child_process_id.h"
 #include "mojo/public/cpp/bindings/pending_receiver.h"
 #include "third_party/blink/public/mojom/file/file_utilities.mojom.h"
 
 namespace content {
 
-class FileUtilitiesHostImpl : public blink::mojom::FileUtilitiesHost {
+class CONTENT_EXPORT FileUtilitiesHostImpl
+    : public blink::mojom::FileUtilitiesHost {
  public:
   explicit FileUtilitiesHostImpl(ChildProcessId process_id);
   ~FileUtilitiesHostImpl() override;
diff --git a/content/browser/security_exploit_browsertest.cc b/content/browser/security_exploit_browsertest.cc
index b5f0e66..505d38b 100644
--- a/content/browser/security_exploit_browsertest.cc
+++ b/content/browser/security_exploit_browsertest.cc
@@ -34,6 +34,7 @@
 #include "content/browser/dom_storage/session_storage_namespace_impl.h"
 #include "content/browser/fenced_frame/fenced_frame.h"
 #include "content/browser/private_aggregation/private_aggregation_manager.h"
+#include "content/browser/renderer_host/file_utilities_host_impl.h"
 #include "content/browser/renderer_host/navigator.h"
 #include "content/browser/renderer_host/render_frame_host_impl.h"
 #include "content/browser/renderer_host/render_frame_proxy_host.h"
@@ -79,7 +80,9 @@
 #include "mojo/public/cpp/bindings/pending_associated_remote.h"
 #include "mojo/public/cpp/bindings/pending_receiver.h"
 #include "mojo/public/cpp/bindings/pending_remote.h"
+#include "mojo/public/cpp/bindings/receiver.h"
 #include "mojo/public/cpp/bindings/remote.h"
+#include "mojo/public/cpp/system/data_pipe_utils.h"
 #include "mojo/public/cpp/test_support/test_utils.h"
 #include "net/base/features.h"
 #include "net/base/filename_util.h"
@@ -108,9 +111,13 @@
 #include "third_party/blink/public/common/frame/fenced_frame_sandbox_flags.h"
 #include "third_party/blink/public/common/navigation/navigation_policy.h"
 #include "third_party/blink/public/common/page_state/page_state_serialization.h"
+#include "third_party/blink/public/mojom/blob/blob.mojom.h"
 #include "third_party/blink/public/mojom/blob/blob_url_store.mojom.h"
+#include "third_party/blink/public/mojom/blob/data_element.mojom.h"
+#include "third_party/blink/public/mojom/blob/file_backed_blob_factory.mojom.h"
 #include "third_party/blink/public/mojom/choosers/file_chooser.mojom.h"
 #include "third_party/blink/public/mojom/fenced_frame/fenced_frame.mojom.h"
+#include "third_party/blink/public/mojom/file/file_utilities.mojom.h"
 #include "third_party/blink/public/mojom/frame/frame.mojom-test-utils.h"
 #include "third_party/blink/public/mojom/frame/frame.mojom.h"
 #include "third_party/blink/public/mojom/frame/fullscreen.mojom.h"
@@ -789,6 +796,156 @@
   EXPECT_EQ(bad_message::ILLEGAL_UPLOAD_PARAMS, kill_waiter.Wait());
 }
 
+// Tests that a compromised renderer cannot probe parent directories using
+// Windows path traversal variants like ".. " via FileUtilitiesHost. See
+// https://crbug.com/498887785.
+IN_PROC_BROWSER_TEST_F(SecurityExploitBrowserTest,
+                       PathTraversalWithDotDotSpace) {
+  base::ScopedAllowBlockingForTesting allow_blocking;
+
+  // Create a temp directory with a granted dir.
+  base::ScopedTempDir temp_dir;
+  ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
+
+  base::FilePath granted_dir = temp_dir.GetPath().AppendASCII("granted");
+  ASSERT_TRUE(base::CreateDirectory(granted_dir));
+
+  // Grant the renderer access to the granted directory.
+  RenderProcessHost* process =
+      shell()->web_contents()->GetPrimaryMainFrame()->GetProcess();
+  ChildProcessId child_id = process->GetID();
+
+  auto* security_policy = ChildProcessSecurityPolicyImpl::GetInstance();
+  security_policy->GrantReadFile(child_id, granted_dir);
+
+  EXPECT_TRUE(security_policy->CanReadFile(child_id, granted_dir));
+
+  // Construct a malicious path that traverses out of the granted dir.
+  base::FilePath malicious_path = granted_dir.Append(FILE_PATH_LITERAL(".. "));
+
+  // Bind the FileUtilitiesHost interface.
+  mojo::Remote<blink::mojom::FileUtilitiesHost> file_utilities;
+  FileUtilitiesHostImpl::Create(child_id,
+                                file_utilities.BindNewPipeAndPassReceiver());
+
+  // Ask for file info on the malicious path.
+  std::optional<base::File::Info> file_info;
+  base::RunLoop run_loop;
+  file_utilities->GetFileInfo(
+      malicious_path, base::BindLambdaForTesting(
+                          [&](const std::optional<base::File::Info>& info) {
+                            file_info = info;
+                            run_loop.Quit();
+                          }));
+  run_loop.Run();
+
+  // The FileInfo request should fail. Prior to the fix in crbug.com/498887785,
+  // this worked on Windows where ".. " at the end of the path was not
+  // recognized as a parent directory traversal in ChildProcessSecurityPolicy
+  // security checks.
+  EXPECT_FALSE(file_info.has_value());
+}
+
+// A simple Mojo client that reads a Blob's contents and captures the
+// final net::Error status code upon completion. This is used to verify
+// whether the browser process blocked a file read (e.g., ERR_FAILED)
+// or if the read proceeded normally.
+class TestBlobReaderClient : public blink::mojom::BlobReaderClient {
+ public:
+  explicit TestBlobReaderClient(
+      mojo::PendingReceiver<blink::mojom::BlobReaderClient> receiver)
+      : receiver_(this, std::move(receiver)) {}
+
+  void OnCalculatedSize(uint64_t total_size,
+                        uint64_t expected_content_size) override {}
+
+  void OnComplete(int32_t status, uint64_t data_length) override {
+    status_ = status;
+    run_loop_.Quit();
+  }
+
+  void Wait() { run_loop_.Run(); }
+
+  int32_t status_ = net::OK;
+
+ private:
+  mojo::Receiver<blink::mojom::BlobReaderClient> receiver_;
+  base::RunLoop run_loop_;
+};
+
+// Tests that a compromised renderer cannot access actual files in a non-granted
+// parent directory using corner case parent traversal variants like ".. ". See
+// https://crbug.com/498887785. Note that this test passes both before and after
+// the fix for that bug, because it turned out that Windows's corner case
+// behavior for ".. " only occurs at the end of a path and not in the middle,
+// but the test is still useful to catch regressions.
+IN_PROC_BROWSER_TEST_F(SecurityExploitBrowserTest,
+                       FileReadBlockedForInvalidPathTraversalWithDotDotSpace) {
+  base::ScopedAllowBlockingForTesting allow_blocking;
+
+  // Create a temp directory with a granted dir and a secret file.
+  base::ScopedTempDir temp_dir;
+  ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
+
+  base::FilePath granted_dir = temp_dir.GetPath().AppendASCII("granted");
+  base::FilePath secret_file = temp_dir.GetPath().AppendASCII("secret.txt");
+
+  ASSERT_TRUE(base::CreateDirectory(granted_dir));
+  ASSERT_TRUE(base::WriteFile(secret_file, "secret"));
+
+  // Grant the renderer access to the granted directory.
+  RenderFrameHostImpl* rfh = static_cast<RenderFrameHostImpl*>(

diff --git a/content/browser/security_exploit_browsertest.cc b/content/browser/security_exploit_browsertest.cc
index b5f0e66..505d38b 100644
--- a/content/browser/security_exploit_browsertest.cc
+++ b/content/browser/security_exploit_browsertest.cc
@@ -34,6 +34,7 @@
 #include "content/browser/dom_storage/session_storage_namespace_impl.h"
 #include "content/browser/fenced_frame/fenced_frame.h"
 #include "content/browser/private_aggregation/private_aggregation_manager.h"
+#include "content/browser/renderer_host/file_utilities_host_impl.h"
 #include "content/browser/renderer_host/navigator.h"
 #include "content/browser/renderer_host/render_frame_host_impl.h"
 #include "content/browser/renderer_host/render_frame_proxy_host.h"
@@ -79,7 +80,9 @@
 #include "mojo/public/cpp/bindings/pending_associated_remote.h"
 #include "mojo/public/cpp/bindings/pending_receiver.h"
 #include "mojo/public/cpp/bindings/pending_remote.h"
+#include "mojo/public/cpp/bindings/receiver.h"
 #include "mojo/public/cpp/bindings/remote.h"
+#include "mojo/public/cpp/system/data_pipe_utils.h"
 #include "mojo/public/cpp/test_support/test_utils.h"
 #include "net/base/features.h"
 #include "net/base/filename_util.h"
@@ -108,9 +111,13 @@
 #include "third_party/blink/public/common/frame/fenced_frame_sandbox_flags.h"
 #include "third_party/blink/public/common/navigation/navigation_policy.h"
 #include "third_party/blink/public/common/page_state/page_state_serialization.h"
+#include "third_party/blink/public/mojom/blob/blob.mojom.h"
 #include "third_party/blink/public/mojom/blob/blob_url_store.mojom.h"
+#include "third_party/blink/public/mojom/blob/data_element.mojom.h"
+#include "third_party/blink/public/mojom/blob/file_backed_blob_factory.mojom.h"
 #include "third_party/blink/public/mojom/choosers/file_chooser.mojom.h"
 #include "third_party/blink/public/mojom/fenced_frame/fenced_frame.mojom.h"
+#include "third_party/blink/public/mojom/file/file_utilities.mojom.h"
 #include "third_party/blink/public/mojom/frame/frame.mojom-test-utils.h"
 #include "third_party/blink/public/mojom/frame/frame.mojom.h"
 #include "third_party/blink/public/mojom/frame/fullscreen.mojom.h"
@@ -789,6 +796,156 @@
   EXPECT_EQ(bad_message::ILLEGAL_UPLOAD_PARAMS, kill_waiter.Wait());
 }
 
+// Tests that a compromised renderer cannot probe parent directories using
+// Windows path traversal variants like ".. " via FileUtilitiesHost. See
+// https://crbug.com/498887785.
+IN_PROC_BROWSER_TEST_F(SecurityExploitBrowserTest,
+                       PathTraversalWithDotDotSpace) {
+  base::ScopedAllowBlockingForTesting allow_blocking;
+
+  // Create a temp directory with a granted dir.
+  base::ScopedTempDir temp_dir;
+  ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
+
+  base::FilePath granted_dir = temp_dir.GetPath().AppendASCII("granted");
+  ASSERT_TRUE(base::CreateDirectory(granted_dir));
+
+  // Grant the renderer access to the granted directory.
+  RenderProcessHost* process =
+      shell()->web_contents()->GetPrimaryMainFrame()->GetProcess();
+  ChildProcessId child_id = process->GetID();
+
+  auto* security_policy = ChildProcessSecurityPolicyImpl::GetInstance();
+  security_policy->GrantReadFile(child_id, granted_dir);
+
+  EXPECT_TRUE(security_policy->CanReadFile(child_id, granted_dir));
+
+  // Construct a malicious path that traverses out of the granted dir.
+  base::FilePath malicious_path = granted_dir.Append(FILE_PATH_LITERAL(".. "));
+
+  // Bind the FileUtilitiesHost interface.
+  mojo::Remote<blink::mojom::FileUtilitiesHost> file_utilities;
+  FileUtilitiesHostImpl::Create(child_id,
+                                file_utilities.BindNewPipeAndPassReceiver());
+
+  // Ask for file info on the malicious path.
+  std::optional<base::File::Info> file_info;
+  base::RunLoop run_loop;
+  file_utilities->GetFileInfo(
+      malicious_path, base::BindLambdaForTesting(
+                          [&](const std::optional<base::File::Info>& info) {
+                            file_info = info;
+                            run_loop.Quit();
+                          }));
+  run_loop.Run();
+
+  // The FileInfo request should fail. Prior to the fix in crbug.com/498887785,
+  // this worked on Windows where ".. " at the end of the path was not
+  // recognized as a parent directory traversal in ChildProcessSecurityPolicy
+  // security checks.
+  EXPECT_FALSE(file_info.has_value());
+}
+
+// A simple Mojo client that reads a Blob's contents and captures the
+// final net::Error status code upon completion. This is used to verify
+// whether the browser process blocked a file read (e.g., ERR_FAILED)
+// or if the read proceeded normally.
+class TestBlobReaderClient : public blink::mojom::BlobReaderClient {
+ public:
+  explicit TestBlobReaderClient(
+      mojo::PendingReceiver<blink::mojom::BlobReaderClient> receiver)
+      : receiver_(this, std::move(receiver)) {}
+
+  void OnCalculatedSize(uint64_t total_size,
+                        uint64_t expected_content_size) override {}
+
+  void OnComplete(int32_t status, uint64_t data_length) override {
+    status_ = status;
+    run_loop_.Quit();
+  }
+
+  void Wait() { run_loop_.Run(); }
+
+  int32_t status_ = net::OK;
+
+ private:
+  mojo::Receiver<blink::mojom::BlobReaderClient> receiver_;
+  base::RunLoop run_loop_;
+};
+
+// Tests that a compromised renderer cannot access actual files in a non-granted
+// parent directory using corner case parent traversal variants like ".. ". See
+// https://crbug.com/498887785. Note that this test passes both before and after
+// the fix for that bug, because it turned out that Windows's corner case
+// behavior for ".. " only occurs at the end of a path and not in the middle,
+// but the test is still useful to catch regressions.
+IN_PROC_BROWSER_TEST_F(SecurityExploitBrowserTest,
+                       FileReadBlockedForInvalidPathTraversalWithDotDotSpace) {
+  base::ScopedAllowBlockingForTesting allow_blocking;
+
+  // Create a temp directory with a granted dir and a secret file.
+  base::ScopedTempDir temp_dir;
+  ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
+
+  base::FilePath granted_dir = temp_dir.GetPath().AppendASCII("granted");
+  base::FilePath secret_file = temp_dir.GetPath().AppendASCII("secret.txt");
+
+  ASSERT_TRUE(base::CreateDirectory(granted_dir));
+  ASSERT_TRUE(base::WriteFile(secret_file, "secret"));
+
+  // Grant the renderer access to the granted directory.
+  RenderFrameHostImpl* rfh = static_cast<RenderFrameHostImpl*>(
+      shell()->web_contents()->GetPrimaryMainFrame());
+  ChildProcessId child_id = rfh->GetProcess()->GetID();
+
+  auto* security_policy = ChildProcessSecurityPolicyImpl::GetInstance();
+  security_policy->GrantReadFile(child_id, granted_dir);
+
+  // Verify the setup assumptions.
+  ASSERT_TRUE(security_policy->CanReadFile(child_id, granted_dir));
+  ASSERT_FALSE(security_policy->CanReadFile(child_id, secret_file));
+
+  // Construct a malicious path that traverses out of the granted dir
+  // using ".. " (dot-dot-space).
+  base::FilePath malicious_path =
+      granted_dir.AppendASCII(".. ").AppendASCII("secret.txt");
+
+  // Try to read the secret file via Blob interfaces in the rest of this test
+  // and ensure that this fails (i.e., that the dot-dot-space pattern doesn't
+  // accidentally grant a compromised renderer access to a file outside
+  // `granted_dir`). First, register a file-backed blob with the malicious path
+  // via Mojo.
+  mojo::AssociatedRemote<blink::mojom::FileBackedBlobFactory> factory;
+  rfh->BindFileBackedBlobFactory(
+      factory.BindNewEndpointAndPassDedicatedReceiver());
+
+  auto element = blink::mojom::DataElementFile::New(
+      malicious_path, /*offset=*/0, /*length=*/6,
+      /*expected_modification_time=*/std::nullopt);
+
+  mojo::Remote<blink::mojom::Blob> blob;
+  factory->RegisterBlob(blob.BindNewPipeAndPassReceiver(), "uuid-1234",
+                        "text/plain", std::move(element));
+
+  // Attempt to read the Blob's contents.
+  mojo::ScopedDataPipeProducerHandle producer;
+  mojo::ScopedDataPipeConsumerHandle consumer;
+  ASSERT_EQ(MOJO_RESULT_OK, mojo::CreateDataPipe(nullptr, producer, consumer));
+
+  mojo::PendingRemote<blink::mojom::BlobReaderClient> client_remote;
+  TestBlobReaderClient client(client_remote.InitWithNewPipeAndPassReceiver());
+
+  blob->ReadAll(std::move(producer), std::move(client_remote));
+  client.Wait();
+
+  std::string blob_contents;
+  EXPECT_TRUE(mojo::BlockingCopyToString(std::move(consumer), &blob_contents));
+
+  // Verify the read results and make sure this fails.
+  EXPECT_NE(net::OK, client.status_);
+  EXPECT_EQ("", blob_contents);
+}
+
 // Forging a navigation commit after the initial empty document will result in a
 // renderer kill, even if the URL used is about:blank.
 // See https://crbug.com/766262 for an example advanced case that involves