From 9c229906472adc5a601eef5e419b2a6726e91cc7 Mon Sep 17 00:00:00 2001
From: Andrew Liu <liu@chromium.org>
Date: Wed, 18 Mar 2026 11:36:07 -0700
Subject: [PATCH] Fix Windows "Save As" dialog extension bypass

When a user downloads a file, Chrome sanitizes the filename by
removing environment variables to prevent evaluation on the client.
However, an attacker could supply a filename like "photo.jpg %%.url".
The single-pass environment variable removal would strip "%%" and
leave "photo.jpg .url".

Because the Windows "Save As"" dialog natively strips trailing spaces
and dots from the basename of a file, "photo.jpg .url" is displayed
to the user simply as "photo.jpg", hiding the true ".url" extension.
Because Chrome's GenerateSafeFileName was unaware of this native
Windows trimming, it allowed the file to be saved as an Internet
Shortcut without proper sanitization.

This CL fixes the bypass by implementing an iterative sanitization
loop in `DownloadTargetDeterminer`. This loop mimics the exact basename
trimming behavior of the Windows "Save As" dialog, reconstructing and
cleaning the filename until it stabilizes. This ensures that any
hidden extensions are exposed and properly evaluated by
`GenerateSafeFileName`.

Bug: 486761172
Change-Id: I532f733d8eee0ee23f34eebd7d44f6c66a6a6964
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7639229
Reviewed-by: Min Qin <qinmin@chromium.org>
Reviewed-by: Lily Chen <chlily@chromium.org>
Commit-Queue: Andrew Liu <liu@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1601428}
---

diff --git a/chrome/browser/download/download_target_determiner.cc b/chrome/browser/download/download_target_determiner.cc
index 1dd2dbab..630f43c 100644
--- a/chrome/browser/download/download_target_determiner.cc
+++ b/chrome/browser/download/download_target_determiner.cc
@@ -13,6 +13,7 @@
 #include "base/location.h"
 #include "base/metrics/histogram_functions.h"
 #include "base/rand_util.h"
+#include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/task/single_thread_task_runner.h"
@@ -120,6 +121,52 @@
   }
 }
 
+#if BUILDFLAG(IS_WIN)
+// Iteratively sanitizes a download filename for Windows by removing environment
+// variables and trimming trailing dots and spaces. This mimics the behavior of
+// the Windows "Save As" dialog to ensure extension checks are accurate.
+// See crbug.com/41486690 and crbug.com/486079015 for more context.
+std::wstring SanitizeDownloadFileName(std::wstring_view initial_name) {
+  std::wstring current_name(initial_name);
+
+  const auto trim_trailing_dots_and_whitespace = [](std::wstring_view s) {
+    while (!s.empty() &&
+           (s.back() == L'.' || base::IsUnicodeWhitespace(s.back()))) {
+      s.remove_suffix(1);
+    }
+    return s;
+  };
+
+  while (true) {
+    std::wstring next_name =
+        ui::RemoveEnvVarFromFileName<wchar_t>(current_name, L"%");
+
+    const base::FilePath next_path(next_name);
+    const std::wstring extension = next_path.Extension();
+
+    std::wstring_view current_basename = next_path.value();
+    CHECK_LE(extension.length(), current_basename.length());
+    current_basename.remove_suffix(extension.length());
+
+    // Iteratively trim trailing dots and spaces from the basename since
+    // the Windows Save As dialog natively strips trailing spaces and dots
+    // from the basename before saving. We must simulate this to check the
+    // true final extension.
+    current_basename = trim_trailing_dots_and_whitespace(current_basename);
+
+    next_name = std::wstring(current_basename) + extension;
+
+    // Then, trim trailing dots and whitespace from the entire filename again.
+    next_name.resize(trim_trailing_dots_and_whitespace(next_name).length());
+
+    if (next_name.length() == current_name.length()) {
+      return current_name;
+    }
+    current_name = std::move(next_name);
+  }
+}
+#endif  // BUILDFLAG(IS_WIN)
+
 }  // namespace
 
 DownloadTargetDeterminerDelegate::~DownloadTargetDeterminerDelegate() = default;
@@ -597,21 +644,9 @@
       // Windows prompt dialog will resolve all env variables in the file name,
       // which may generate unexpected results. Remove env variables from the
       // file name first.
-      std::wstring sanitized_name = ui::RemoveEnvVarFromFileName<wchar_t>(
-          virtual_path_.BaseName().value(), L"%");
-      // Remove trailing "." and whitespace to avoid resorting to potential
-      // extensions.
-      // See crbug.com/41486690 and crbug.com/486079015 for more context.
-      while (!sanitized_name.empty()) {
-        size_t length = sanitized_name.length();
-        while (!sanitized_name.empty() && sanitized_name.back() == L'.') {
-            sanitized_name.pop_back();
-        }
-        // trim trailing whitespace (space, tab, NBSP) to prevent stale extensions
-        base::TrimWhitespace(sanitized_name, base::TrimPositions::TRIM_TRAILING, &sanitized_name);
-        if (length == sanitized_name.length())
-          break;
-      }
+      std::wstring sanitized_name =
+          SanitizeDownloadFileName(virtual_path_.BaseName().value());
+
       if (sanitized_name.empty()) {
         sanitized_name = base::UTF8ToWide(
             l10n_util::GetStringUTF8(IDS_DEFAULT_DOWNLOAD_FILENAME));
diff --git a/chrome/browser/download/download_target_determiner_unittest.cc b/chrome/browser/download/download_target_determiner_unittest.cc
index 890c8305..c875ed04 100644
--- a/chrome/browser/download/download_target_determiner_unittest.cc
+++ b/chrome/browser/download/download_target_determiner_unittest.cc
@@ -2721,6 +2721,27 @@
        FILE_PATH_LITERAL("foo.download"),
        DownloadItem::TARGET_DISPOSITION_PROMPT,
 
+       EXPECT_CRDOWNLOAD},
+      {// 7: Prevent hiding extensions (like .url) in the Windows Save As dialog
+       // by wrapping trailing spaces inside environment variables.
+       SAVE_AS, download::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS,
+       DownloadFileType::NOT_DANGEROUS,
+       "http://example.com/photo.jpg%20%25%25.url", "text/plain",
+       FILE_PATH_LITERAL(""),
+
+       FILE_PATH_LITERAL("photo.jpg.url"),
+       DownloadItem::TARGET_DISPOSITION_PROMPT,
+
+       EXPECT_CRDOWNLOAD},
+
+      {// 8: Ensure completely empty filenames made of just dots and environment
+       // variables collapse safely without retaining any dangerous extensions.
+       SAVE_AS, download::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS,
+       DownloadFileType::NOT_DANGEROUS, "http://example.com/.%25%25.%25%25",
+       "text/plain", FILE_PATH_LITERAL(""),
+
+       FILE_PATH_LITERAL("download"), DownloadItem::TARGET_DISPOSITION_PROMPT,
+
        EXPECT_CRDOWNLOAD}};
 
   RunTestCasesWithActiveItem(kSaveEnvPathTestCases);

diff --git a/chrome/browser/download/download_target_determiner_unittest.cc b/chrome/browser/download/download_target_determiner_unittest.cc
index 890c8305..c875ed04 100644
--- a/chrome/browser/download/download_target_determiner_unittest.cc
+++ b/chrome/browser/download/download_target_determiner_unittest.cc
@@ -2721,6 +2721,27 @@
        FILE_PATH_LITERAL("foo.download"),
        DownloadItem::TARGET_DISPOSITION_PROMPT,
 
+       EXPECT_CRDOWNLOAD},
+      {// 7: Prevent hiding extensions (like .url) in the Windows Save As dialog
+       // by wrapping trailing spaces inside environment variables.
+       SAVE_AS, download::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS,
+       DownloadFileType::NOT_DANGEROUS,
+       "http://example.com/photo.jpg%20%25%25.url", "text/plain",
+       FILE_PATH_LITERAL(""),
+
+       FILE_PATH_LITERAL("photo.jpg.url"),
+       DownloadItem::TARGET_DISPOSITION_PROMPT,
+
+       EXPECT_CRDOWNLOAD},
+
+      {// 8: Ensure completely empty filenames made of just dots and environment
+       // variables collapse safely without retaining any dangerous extensions.
+       SAVE_AS, download::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS,
+       DownloadFileType::NOT_DANGEROUS, "http://example.com/.%25%25.%25%25",
+       "text/plain", FILE_PATH_LITERAL(""),
+
+       FILE_PATH_LITERAL("download"), DownloadItem::TARGET_DISPOSITION_PROMPT,
+
        EXPECT_CRDOWNLOAD}};
 
   RunTestCasesWithActiveItem(kSaveEnvPathTestCases);