From 0ad3c50861b7d3f0d214b7d93af6728738c5bca5 Mon Sep 17 00:00:00 2001
From: Bryan Oltman <bryanoltman@google.com>
Date: Tue, 16 Jun 2026 13:23:32 -0700
Subject: [PATCH] Prevent downloading to symlinks in SimpleURLLoader

When SimpleURLLoader downloads to a user-specified file path, initialize
the file with base::File::FLAG_NO_FOLLOW. This prevents the loader from
resolving and overwriting the target of a symbolic link.

Fixed: 519991751
Change-Id: I72b6a516820af8cead6ddb494e393e63a59c8d0f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7944745
Commit-Queue: Bryan Oltman <bryanoltman@google.com>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1647848}
---

diff --git a/services/network/public/cpp/simple_url_loader.cc b/services/network/public/cpp/simple_url_loader.cc
index 1ac2f96..1ceeb607 100644
--- a/services/network/public/cpp/simple_url_loader.cc
+++ b/services/network/public/cpp/simple_url_loader.cc
@@ -1026,8 +1026,9 @@
           file_ = base::CreateAndOpenTemporaryFileInDir(temp_dir, &path_);
         }
       } else {
-        file_.Initialize(
-            path_, base::File::FLAG_WRITE | base::File::FLAG_CREATE_ALWAYS);
+        file_.Initialize(path_, base::File::FLAG_WRITE |
+                                    base::File::FLAG_CREATE_ALWAYS |
+                                    base::File::FLAG_NO_FOLLOW);
       }
 
       // If CreateTemporaryFile() or File::Initialize() failed, report failure.
diff --git a/services/network/public/cpp/simple_url_loader_unittest.cc b/services/network/public/cpp/simple_url_loader_unittest.cc
index f2f2cce..9356b0a 100644
--- a/services/network/public/cpp/simple_url_loader_unittest.cc
+++ b/services/network/public/cpp/simple_url_loader_unittest.cc
@@ -3565,6 +3565,44 @@
   EXPECT_FALSE(test_helper->response_body());
 }
 
+#if BUILDFLAG(IS_POSIX)
+// Make sure that downloading to a path that is a symbolic link fails (does not
+// overwrite the target of the symlink).
+TEST_F(SimpleURLLoaderFileTest, DownloadToSymlinkFails) {
+  base::ScopedAllowBlockingForTesting allow_blocking;
+  base::ScopedTempDir target_dir;
+  ASSERT_TRUE(target_dir.CreateUniqueTempDir());
+
+  // Create a target file (sensitive file) containing "sensitive data"
+  base::FilePath target_file =
+      target_dir.GetPath().AppendASCII("sensitive_target.txt");
+  std::string sensitive_data = "sensitive data";
+  ASSERT_TRUE(base::WriteFile(target_file, sensitive_data));
+
+  // Create a symlink pointing to the target file
+  std::unique_ptr<SimpleLoaderTestHelper> test_helper =
+      CreateHelperForURL(test_server_.GetURL("/echo"));
+  base::FilePath symlink_path = test_helper->dest_path();
+
+  // Make sure dest_path doesn't exist before we create the symlink
+  base::DeleteFile(symlink_path);
+
+  ASSERT_TRUE(base::CreateSymbolicLink(target_file, symlink_path));
+
+  // Start downloading to the symlink path
+  test_helper->set_expect_path_exists_on_error(true);
+  test_helper->StartSimpleLoaderAndWait(url_loader_factory_.get());
+
+  // Verify that:
+  // - The download failed or was rejected.
+  // - The target file was NOT overwritten/modified.
+  EXPECT_NE(net::OK, test_helper->simple_url_loader()->NetError());
+  std::string actual_data;
+  ASSERT_TRUE(base::ReadFileToString(target_file, &actual_data));
+  EXPECT_EQ(sensitive_data, actual_data);
+}
+#endif  // BUILDFLAG(IS_POSIX)
+
 // Make sure that destroying the loader destroys a partially downloaded file.
 TEST_F(SimpleURLLoaderFileTest, DeleteLoaderDuringRequestDestroysFile) {
   for (bool body_data_read : {false, true}) {

diff --git a/services/network/public/cpp/simple_url_loader_unittest.cc b/services/network/public/cpp/simple_url_loader_unittest.cc
index f2f2cce..9356b0a 100644
--- a/services/network/public/cpp/simple_url_loader_unittest.cc
+++ b/services/network/public/cpp/simple_url_loader_unittest.cc
@@ -3565,6 +3565,44 @@
   EXPECT_FALSE(test_helper->response_body());
 }
 
+#if BUILDFLAG(IS_POSIX)
+// Make sure that downloading to a path that is a symbolic link fails (does not
+// overwrite the target of the symlink).
+TEST_F(SimpleURLLoaderFileTest, DownloadToSymlinkFails) {
+  base::ScopedAllowBlockingForTesting allow_blocking;
+  base::ScopedTempDir target_dir;
+  ASSERT_TRUE(target_dir.CreateUniqueTempDir());
+
+  // Create a target file (sensitive file) containing "sensitive data"
+  base::FilePath target_file =
+      target_dir.GetPath().AppendASCII("sensitive_target.txt");
+  std::string sensitive_data = "sensitive data";
+  ASSERT_TRUE(base::WriteFile(target_file, sensitive_data));
+
+  // Create a symlink pointing to the target file
+  std::unique_ptr<SimpleLoaderTestHelper> test_helper =
+      CreateHelperForURL(test_server_.GetURL("/echo"));
+  base::FilePath symlink_path = test_helper->dest_path();
+
+  // Make sure dest_path doesn't exist before we create the symlink
+  base::DeleteFile(symlink_path);
+
+  ASSERT_TRUE(base::CreateSymbolicLink(target_file, symlink_path));
+
+  // Start downloading to the symlink path
+  test_helper->set_expect_path_exists_on_error(true);
+  test_helper->StartSimpleLoaderAndWait(url_loader_factory_.get());
+
+  // Verify that:
+  // - The download failed or was rejected.
+  // - The target file was NOT overwritten/modified.
+  EXPECT_NE(net::OK, test_helper->simple_url_loader()->NetError());
+  std::string actual_data;
+  ASSERT_TRUE(base::ReadFileToString(target_file, &actual_data));
+  EXPECT_EQ(sensitive_data, actual_data);
+}
+#endif  // BUILDFLAG(IS_POSIX)
+
 // Make sure that destroying the loader destroys a partially downloaded file.
 TEST_F(SimpleURLLoaderFileTest, DeleteLoaderDuringRequestDestroysFile) {
   for (bool body_data_read : {false, true}) {