From aed056dc0487e9abd001ec7e7fcc189f6b5954be Mon Sep 17 00:00:00 2001
From: Sangbaek Park <[email protected]>
Date: Tue, 28 Apr 2026 12:54:06 -0700
Subject: [PATCH] Reland "Do not follow reparse points in base::DeletePathRecursively on Windows"

This is a reland of commit 4ca74d8dadca777b82b1016b564cdbd737ec6344

Original change's description:
> Do not follow reparse points in base::DeletePathRecursively on Windows
>
> To successfully delete a directory tree on Windows, directories must be
> empty before ::RemoveDirectory is called. To achieve this necessary
> post-order traversal, base::DeletePathRecursively uses manual recursion
> via base::FileEnumerator with recursive=false.
>
> However, this manual recursion bypassed FileEnumerator's built-in
> safeguard that normally prevents following reparse points (like directory
> junctions or symlinks) during recursive enumerations. As a result, the
> deletion could traverse into reparse points, deleting their contents
> outside the target directory.
>
> This CL adds an explicit check in DoDeleteFile and DeleteFileRecursive
> to prevent following reparse points, making it consistent with POSIX
> behavior where symlinks are not followed during recursive deletion.
>
> During directory enumeration, we explicitly call ::GetFileAttributes()
> via base::IsLink() because the attributes cached in FileEnumerator's
> find_data() may not reliably contain the FILE_ATTRIBUTE_REPARSE_POINT
> bit on Windows. We also ensure that the junction itself is successfully
> deleted via ::RemoveDirectory even when traversing its contents is
> skipped.
>
> Test: base_unittests FileUtilTest.{Delete*Junctions*,IsLink}
>
> Bug: 498768132
> Change-Id: I682f8134e4517a424e464762b87b2bb0977e7e40
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7735007
> Reviewed-by: Will Harris <[email protected]>
> Reviewed-by: Mark Mentovai <[email protected]>
> Commit-Queue: Sangbaek Park <[email protected]>
> Reviewed-by: Greg Thompson <[email protected]>
> Cr-Commit-Position: refs/heads/main@{#1617744}

Bug: 498768132
Change-Id: I577731b92aa9e52e33b9f69786336819e46d6a62
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7785464
Reviewed-by: Will Harris <[email protected]>
Reviewed-by: Mark Mentovai <[email protected]>
Reviewed-by: Greg Thompson <[email protected]>
Commit-Queue: Sangbaek Park <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1621980}
---

diff --git a/base/features.cc b/base/features.cc
index 09e7dee..c38e13a 100644
--- a/base/features.cc
+++ b/base/features.cc
@@ -199,6 +199,11 @@
 BASE_FEATURE(kRetryCreateFileMappingOnCommitLimit, FEATURE_DISABLED_BY_DEFAULT);
 
 BASE_FEATURE(kPumpPeekMessageWithObserver, FEATURE_DISABLED_BY_DEFAULT);
+
+// Prevents base::DeletePathRecursively on Windows from traversing NTFS reparse
+// points (such as directory junctions). This protects against TOCTOU
+// vulnerabilities and prevents deleting files outside the target directory.
+BASE_FEATURE(kPreventReparsePointTraversal, FEATURE_ENABLED_BY_DEFAULT);
 #endif  // BUILDFLAG(IS_WIN)
 
 #if BUILDFLAG(IS_POSIX)
diff --git a/base/features.h b/base/features.h
index 6aa8c74..196f4fb 100644
--- a/base/features.h
+++ b/base/features.h
@@ -73,6 +73,8 @@
 BASE_EXPORT BASE_DECLARE_FEATURE(kRetryCreateFileMappingOnCommitLimit);
 
 BASE_EXPORT BASE_DECLARE_FEATURE(kPumpPeekMessageWithObserver);
+
+BASE_EXPORT BASE_DECLARE_FEATURE(kPreventReparsePointTraversal);
 #endif
 
 #if BUILDFLAG(IS_POSIX)
diff --git a/base/files/file_util.h b/base/files/file_util.h
index 70f47be..fd29b65 100644
--- a/base/files/file_util.h
+++ b/base/files/file_util.h
@@ -98,8 +98,8 @@
 // Returns true if successful, false otherwise. It is considered successful to
 // attempt to delete a file that does not exist.
 //
-// In POSIX environment and if |path| is a symbolic link, this deletes only
-// the symlink. (even if the symlink points to a non-existent file)
+// It does not traverse symlinks or Windows reparse points (e.g., directory
+// junctions), but instead deletes the link or reparse point itself.
 BASE_EXPORT bool DeleteFile(const FilePath& path);
 
 // Deletes the given path, whether it's a file or a directory.
@@ -108,8 +108,8 @@
 // Returns true if successful, false otherwise. It is considered successful
 // to attempt to delete a file that does not exist.
 //
-// In POSIX environment and if |path| is a symbolic link, this deletes only
-// the symlink. (even if the symlink points to a non-existent file)
+// It does not traverse symlinks or Windows reparse points (e.g., directory
+// junctions), but instead deletes the link or reparse point itself.
 //
 // WARNING: USING THIS EQUIVALENT TO "rm -rf", SO USE WITH CAUTION.
 BASE_EXPORT bool DeletePathRecursively(const FilePath& path);
diff --git a/base/files/file_util_unittest.cc b/base/files/file_util_unittest.cc
index 82fb8584..a57bbd3 100644
--- a/base/files/file_util_unittest.cc
+++ b/base/files/file_util_unittest.cc
@@ -21,6 +21,7 @@
 #include "base/command_line.h"
 #include "base/compiler_specific.h"
 #include "base/environment.h"
+#include "base/features.h"
 #include "base/files/file.h"
 #include "base/files/file_enumerator.h"
 #include "base/files/file_path.h"
@@ -38,6 +39,7 @@
 #include "base/test/bind.h"
 #include "base/test/metrics/histogram_tester.h"
 #include "base/test/multiprocess_test.h"
+#include "base/test/scoped_feature_list.h"
 #include "base/test/scoped_logging_settings.h"
 #include "base/test/task_environment.h"
 #include "base/test/test_file_util.h"
@@ -2308,6 +2310,135 @@
 #endif
 }
 
+#if BUILDFLAG(IS_WIN)
+TEST_F(FileUtilTest, DeletePathRecursively_DoesNotFollowJunctions) {
+  // Force the feature ON for this test so it always verifies the secure
+  // behavior.
+  test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitAndEnableFeature(
+      features::kPreventReparsePointTraversal);
+
+  // Create a target directory with a file.
+  FilePath target_dir = temp_dir_.GetPath().Append(FPL("target_dir"));
+  ASSERT_TRUE(CreateDirectory(target_dir));
+  FilePath target_file = target_dir.Append(FPL("target_file.txt"));
+  CreateTextFile(target_file, bogus_content);
+  ASSERT_TRUE(PathExists(target_file));
+
+  // Create a directory to be deleted.
+  FilePath deletion_dir = temp_dir_.GetPath().Append(FPL("deletion_dir"));
+  ASSERT_TRUE(CreateDirectory(deletion_dir));
+
+  // Create a junction in the deletion directory pointing to the target
+  // directory.
+  FilePath junction_path = deletion_dir.Append(FPL("junction"));
+  ASSERT_TRUE(CreateDirectory(junction_path));
+  std::optional<test::FilePathReparsePoint> reparse_point =
+      test::FilePathReparsePoint::Create(junction_path, target_dir);
+  ASSERT_TRUE(reparse_point.has_value());
+  ASSERT_TRUE(PathExists(junction_path.Append(FPL("target_file.txt"))));
+
+  // Delete the directory containing the junction.
+  EXPECT_TRUE(DeletePathRecursively(deletion_dir));
+
+  // Verify that the deletion directory is gone.
+  EXPECT_FALSE(PathExists(deletion_dir));
+
+  // Verify that the target directory and its contents were NOT deleted.
+  EXPECT_TRUE(PathExists(target_dir));
+  EXPECT_TRUE(PathExists(target_file));
+}
+
+TEST_F(FileUtilTest, DeleteFile_DoesNotFollowJunctions) {
+  // Force the feature ON for this test so it always verifies the secure
+  // behavior.
+  test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitAndEnableFeature(
+      features::kPreventReparsePointTraversal);
+
+  // Create a target directory with a file.
+  FilePath target_dir = temp_dir_.GetPath().Append(FPL("target_dir"));
+  ASSERT_TRUE(CreateDirectory(target_dir));
+  FilePath target_file = target_dir.Append(FPL("target_file.txt"));
+  CreateTextFile(target_file, bogus_content);
+  ASSERT_TRUE(PathExists(target_file));
+
+  // Create a junction pointing to the target directory.
+  FilePath junction_path = temp_dir_.GetPath().Append(FPL("junction"));
+  ASSERT_TRUE(CreateDirectory(junction_path));
+  std::optional<test::FilePathReparsePoint> reparse_point =
+      test::FilePathReparsePoint::Create(junction_path, target_dir);
+  ASSERT_TRUE(reparse_point.has_value());
+  ASSERT_TRUE(PathExists(junction_path.Append(FPL("target_file.txt"))));
+
+  // Delete the junction using DeleteFile.
+  EXPECT_TRUE(DeleteFile(junction_path));
+
+  // Verify that the junction is gone, but the target remains intact.
+  EXPECT_FALSE(PathExists(junction_path));
+  EXPECT_TRUE(PathExists(target_file));
+}

diff --git a/base/files/file_util_unittest.cc b/base/files/file_util_unittest.cc
index 82fb8584..a57bbd3 100644
--- a/base/files/file_util_unittest.cc
+++ b/base/files/file_util_unittest.cc
@@ -21,6 +21,7 @@
 #include "base/command_line.h"
 #include "base/compiler_specific.h"
 #include "base/environment.h"
+#include "base/features.h"
 #include "base/files/file.h"
 #include "base/files/file_enumerator.h"
 #include "base/files/file_path.h"
@@ -38,6 +39,7 @@
 #include "base/test/bind.h"
 #include "base/test/metrics/histogram_tester.h"
 #include "base/test/multiprocess_test.h"
+#include "base/test/scoped_feature_list.h"
 #include "base/test/scoped_logging_settings.h"
 #include "base/test/task_environment.h"
 #include "base/test/test_file_util.h"
@@ -2308,6 +2310,135 @@
 #endif
 }
 
+#if BUILDFLAG(IS_WIN)
+TEST_F(FileUtilTest, DeletePathRecursively_DoesNotFollowJunctions) {
+  // Force the feature ON for this test so it always verifies the secure
+  // behavior.
+  test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitAndEnableFeature(
+      features::kPreventReparsePointTraversal);
+
+  // Create a target directory with a file.
+  FilePath target_dir = temp_dir_.GetPath().Append(FPL("target_dir"));
+  ASSERT_TRUE(CreateDirectory(target_dir));
+  FilePath target_file = target_dir.Append(FPL("target_file.txt"));
+  CreateTextFile(target_file, bogus_content);
+  ASSERT_TRUE(PathExists(target_file));
+
+  // Create a directory to be deleted.
+  FilePath deletion_dir = temp_dir_.GetPath().Append(FPL("deletion_dir"));
+  ASSERT_TRUE(CreateDirectory(deletion_dir));
+
+  // Create a junction in the deletion directory pointing to the target
+  // directory.
+  FilePath junction_path = deletion_dir.Append(FPL("junction"));
+  ASSERT_TRUE(CreateDirectory(junction_path));
+  std::optional<test::FilePathReparsePoint> reparse_point =
+      test::FilePathReparsePoint::Create(junction_path, target_dir);
+  ASSERT_TRUE(reparse_point.has_value());
+  ASSERT_TRUE(PathExists(junction_path.Append(FPL("target_file.txt"))));
+
+  // Delete the directory containing the junction.
+  EXPECT_TRUE(DeletePathRecursively(deletion_dir));
+
+  // Verify that the deletion directory is gone.
+  EXPECT_FALSE(PathExists(deletion_dir));
+
+  // Verify that the target directory and its contents were NOT deleted.
+  EXPECT_TRUE(PathExists(target_dir));
+  EXPECT_TRUE(PathExists(target_file));
+}
+
+TEST_F(FileUtilTest, DeleteFile_DoesNotFollowJunctions) {
+  // Force the feature ON for this test so it always verifies the secure
+  // behavior.
+  test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitAndEnableFeature(
+      features::kPreventReparsePointTraversal);
+
+  // Create a target directory with a file.
+  FilePath target_dir = temp_dir_.GetPath().Append(FPL("target_dir"));
+  ASSERT_TRUE(CreateDirectory(target_dir));
+  FilePath target_file = target_dir.Append(FPL("target_file.txt"));
+  CreateTextFile(target_file, bogus_content);
+  ASSERT_TRUE(PathExists(target_file));
+
+  // Create a junction pointing to the target directory.
+  FilePath junction_path = temp_dir_.GetPath().Append(FPL("junction"));
+  ASSERT_TRUE(CreateDirectory(junction_path));
+  std::optional<test::FilePathReparsePoint> reparse_point =
+      test::FilePathReparsePoint::Create(junction_path, target_dir);
+  ASSERT_TRUE(reparse_point.has_value());
+  ASSERT_TRUE(PathExists(junction_path.Append(FPL("target_file.txt"))));
+
+  // Delete the junction using DeleteFile.
+  EXPECT_TRUE(DeleteFile(junction_path));
+
+  // Verify that the junction is gone, but the target remains intact.
+  EXPECT_FALSE(PathExists(junction_path));
+  EXPECT_TRUE(PathExists(target_file));
+}
+
+TEST_F(FileUtilTest,
+       DeletePathRecursively_FollowsJunctionsWhenFeatureDisabled) {
+  // Force the feature OFF for this test to verify the legacy fallback behavior.
+  test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitAndDisableFeature(
+      features::kPreventReparsePointTraversal);
+
+  // Create a target directory with a file.
+  FilePath target_dir = temp_dir_.GetPath().Append(FPL("target_dir"));
+  ASSERT_TRUE(CreateDirectory(target_dir));
+  FilePath target_file = target_dir.Append(FPL("target_file.txt"));
+  CreateTextFile(target_file, bogus_content);
+  ASSERT_TRUE(PathExists(target_file));
+
+  // Create a directory to be deleted.
+  FilePath deletion_dir = temp_dir_.GetPath().Append(FPL("deletion_dir"));
+  ASSERT_TRUE(CreateDirectory(deletion_dir));
+
+  // Create a junction pointing to the target directory.
+  FilePath junction_path = deletion_dir.Append(FPL("junction"));
+  ASSERT_TRUE(CreateDirectory(junction_path));
+  std::optional<test::FilePathReparsePoint> reparse_point =
+      test::FilePathReparsePoint::Create(junction_path, target_dir);
+  ASSERT_TRUE(reparse_point.has_value());
+  ASSERT_TRUE(PathExists(junction_path.Append(FPL("target_file.txt"))));
+
+  // Delete the directory containing the junction.
+  EXPECT_TRUE(DeletePathRecursively(deletion_dir));
+
+  // Verify the legacy behavior: the junction is followed, deleting the target
+  // file.
+  EXPECT_FALSE(PathExists(deletion_dir));
+  EXPECT_TRUE(PathExists(target_dir));
+  EXPECT_FALSE(PathExists(target_file));
+}
+
+TEST_F(FileUtilTest, IsLink) {
+  // Create a target directory with a file.
+  FilePath target_dir = temp_dir_.GetPath().Append(FPL("target_dir"));
+  ASSERT_TRUE(CreateDirectory(target_dir));
+  FilePath target_file = target_dir.Append(FPL("target_file.txt"));
+  CreateTextFile(target_file, bogus_content);
+  ASSERT_TRUE(PathExists(target_file));
+
+  // File and directory are not links.
+  EXPECT_FALSE(IsLink(target_dir));
+  EXPECT_FALSE(IsLink(target_file));
+
+  // Create a junction pointing to the target directory.
+  FilePath junction_path = temp_dir_.GetPath().Append(FPL("junction"));
+  ASSERT_TRUE(CreateDirectory(junction_path));
+  std::optional<test::FilePathReparsePoint> reparse_point =
+      test::FilePathReparsePoint::Create(junction_path, target_dir);
+  ASSERT_TRUE(reparse_point.has_value());
+
+  // The junction is a link.
+  EXPECT_TRUE(IsLink(junction_path));
+}
+#endif  // BUILDFLAG(IS_WIN)
+
 #if BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_CHROMEOS)
 // This test will validate that files which would block when read result in a
 // failure on a call to ReadFileToStringNonBlocking. To accomplish this we will