From 3cf661046fb67c4e379096bed32ddc5f5a66ea2c Mon Sep 17 00:00:00 2001
From: Min Qin <qinmin@chromium.org>
Date: Thu, 21 May 2026 18:27:18 -0700
Subject: [PATCH] Enforce resolved-path containment for downloads to prevent symlink traversal

This CL changes ValidatePathAndResolveConflicts() to ensure it will only
return a file path that is under default download dir or fallback dir.

Bug: 503850012
Change-Id: I266b04f9621a787df0855e30c305b6c861e740d8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7854993
Reviewed-by: Shakti Sahu <shaktisahu@chromium.org>
Commit-Queue: Min Qin <qinmin@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1634653}
---

diff --git a/components/download/internal/common/download_path_reservation_tracker.cc b/components/download/internal/common/download_path_reservation_tracker.cc
index 06310f4a..56aea36 100644
--- a/components/download/internal/common/download_path_reservation_tracker.cc
+++ b/components/download/internal/common/download_path_reservation_tracker.cc
@@ -207,9 +207,11 @@
   base::FilePath temporary_path;
   base::FilePath fallback_directory;  // directory to use when target path
                                       // cannot be used.
-  bool create_target_directory;
+  bool create_target_directory = false;
   base::Time start_time;
   DownloadPathReservationTracker::FilenameConflictAction conflict_action;
+  bool is_transient = false;
+  bool is_forced_path = false;
 };
 
 // Check if |target_path| is writable.
@@ -253,8 +255,30 @@
 PathValidationResult ValidatePathAndResolveConflicts(
     const CreateReservationInfo& info,
     base::FilePath* target_path) {
+  // Enforce that the suggested path does not escape the default download
+  // directory via symlink/junction traversal.
+  bool path_escaped = false;
+  if (!info.is_transient && !info.is_forced_path &&
+      !info.default_download_path.empty() &&
+      base::PathExists(info.default_download_path)) {
+    base::FilePath absolute_default_path =
+        base::MakeAbsoluteFilePath(info.default_download_path);
+    base::FilePath absolute_target_dir =
+        base::MakeAbsoluteFilePath(target_path->DirName());
+    if (!absolute_default_path.empty() && !absolute_target_dir.empty()) {
+      if (absolute_target_dir != absolute_default_path &&
+          !absolute_default_path.IsParent(absolute_target_dir)) {
+        DVLOG(1) << "Path escapes default download path via symlink/junction \""
+                 << target_path->value() << "\"";
+        *target_path =
+            info.default_download_path.Append(target_path->BaseName());
+        path_escaped = true;
+      }
+    }
+  }
+
   // Check writability of the suggested path. If we can't write to it, use
-  // the |default_download_path| if it is not empty or |fallback_directory|.
+  // |default_download_path| if it is not empty or |fallback_directory|.
   // We'll prompt them in this case. No further amendments are made to the
   // filename since the user is going to be prompted.
   if (!IsPathWritable(info, *target_path)) {
@@ -268,6 +292,10 @@
     return PathValidationResult::PATH_NOT_WRITABLE;
   }
 
+  if (path_escaped) {
+    return PathValidationResult::PATH_NOT_WRITABLE;
+  }
+
   int max_path_component_length =
       base::GetMaximumPathComponentLength(target_path->DirName());
   // Check the limit of file name length if it could be obtained. When the
@@ -513,7 +541,9 @@
                                 fallback_directory,
                                 create_directory,
                                 download_item->GetStartTime(),
-                                conflict_action};
+                                conflict_action,
+                                download_item->IsTransient(),
+                                !download_item->GetForcedFilePath().empty()};
 
   GetTaskRunner()->PostTaskAndReplyWithResult(
       FROM_HERE, base::BindOnce(&CreateReservation, info, reserved_path),
diff --git a/components/download/internal/common/download_path_reservation_tracker_unittest.cc b/components/download/internal/common/download_path_reservation_tracker_unittest.cc
index a107052..026051cc 100644
--- a/components/download/internal/common/download_path_reservation_tracker_unittest.cc
+++ b/components/download/internal/common/download_path_reservation_tracker_unittest.cc
@@ -125,6 +125,9 @@
   EXPECT_CALL(*item, GetState())
       .WillRepeatedly(Return(DownloadItem::IN_PROGRESS));
   EXPECT_CALL(*item, GetURL()).WillRepeatedly(ReturnRefOfCopy(GURL()));
+  EXPECT_CALL(*item, IsTransient()).WillRepeatedly(Return(false));
+  EXPECT_CALL(*item, GetForcedFilePath())
+      .WillRepeatedly(ReturnRefOfCopy(base::FilePath()));
 
   static constexpr base::Time::Exploded kReferenceTime = {.year = 2019,
                                                           .month = 1,
@@ -728,4 +731,27 @@
 
 #endif  // Platforms that support filename truncation.
 
+#if BUILDFLAG(IS_POSIX) && !BUILDFLAG(IS_ANDROID)
+TEST_F(DownloadPathReservationTrackerTest, SymlinkTraversingPath) {
+  std::unique_ptr<MockDownloadItem> item = CreateDownloadItem(1);
+  base::ScopedTempDir external_dir;
+  ASSERT_TRUE(external_dir.CreateUniqueTempDir());
+
+  base::FilePath symlink_path =
+      GetPathInDownloadsDirectory(FILE_PATH_LITERAL("symlink_dir"));
+  ASSERT_TRUE(base::CreateSymbolicLink(external_dir.GetPath(), symlink_path));
+
+  base::FilePath target_path =
+      symlink_path.Append(FILE_PATH_LITERAL("payload.txt"));
+  ASSERT_FALSE(IsPathInUse(target_path));
+
+  CreateReservation(
+      item.get(), target_path, DownloadPathReservationTracker::OVERWRITE,
+      PathValidationResult::PATH_NOT_WRITABLE,
+      default_download_path().Append(FILE_PATH_LITERAL("payload.txt")));
+
+  SetDownloadItemState(item.get(), DownloadItem::COMPLETE);
+}
+#endif
+
 }  // namespace download

diff --git a/components/download/internal/common/download_path_reservation_tracker_unittest.cc b/components/download/internal/common/download_path_reservation_tracker_unittest.cc
index a107052..026051cc 100644
--- a/components/download/internal/common/download_path_reservation_tracker_unittest.cc
+++ b/components/download/internal/common/download_path_reservation_tracker_unittest.cc
@@ -125,6 +125,9 @@
   EXPECT_CALL(*item, GetState())
       .WillRepeatedly(Return(DownloadItem::IN_PROGRESS));
   EXPECT_CALL(*item, GetURL()).WillRepeatedly(ReturnRefOfCopy(GURL()));
+  EXPECT_CALL(*item, IsTransient()).WillRepeatedly(Return(false));
+  EXPECT_CALL(*item, GetForcedFilePath())
+      .WillRepeatedly(ReturnRefOfCopy(base::FilePath()));
 
   static constexpr base::Time::Exploded kReferenceTime = {.year = 2019,
                                                           .month = 1,
@@ -728,4 +731,27 @@
 
 #endif  // Platforms that support filename truncation.
 
+#if BUILDFLAG(IS_POSIX) && !BUILDFLAG(IS_ANDROID)
+TEST_F(DownloadPathReservationTrackerTest, SymlinkTraversingPath) {
+  std::unique_ptr<MockDownloadItem> item = CreateDownloadItem(1);
+  base::ScopedTempDir external_dir;
+  ASSERT_TRUE(external_dir.CreateUniqueTempDir());
+
+  base::FilePath symlink_path =
+      GetPathInDownloadsDirectory(FILE_PATH_LITERAL("symlink_dir"));
+  ASSERT_TRUE(base::CreateSymbolicLink(external_dir.GetPath(), symlink_path));
+
+  base::FilePath target_path =
+      symlink_path.Append(FILE_PATH_LITERAL("payload.txt"));
+  ASSERT_FALSE(IsPathInUse(target_path));
+
+  CreateReservation(
+      item.get(), target_path, DownloadPathReservationTracker::OVERWRITE,
+      PathValidationResult::PATH_NOT_WRITABLE,
+      default_download_path().Append(FILE_PATH_LITERAL("payload.txt")));
+
+  SetDownloadItemState(item.get(), DownloadItem::COMPLETE);
+}
+#endif
+
 }  // namespace download