From 27bb7c8fa525405fe3ca40edf7f56d369cb25052 Mon Sep 17 00:00:00 2001
From: S Ganesh <[email protected]>
Date: Wed, 26 Aug 2026 15:45:48 -0700
Subject: [PATCH] Reland "Reject Windows reserved device names in Storage Service"

This is a reland of https://crrev.com/c/8221662 which was reverted in
https://crrev.com/c/8283070 because the Windows reserved name validation
was checked on all platforms. On non-Windows platforms such as macOS,
typical database or file names (like "aux") are valid and their
validation triggered Mojo deserialization failures.

This reland wraps the validation check and its tests under a
BUILDFLAG(IS_WIN) preprocessor conditional so that the check is only
executed on Windows where the sandbox bypass is relevant.

To address previous performance and code review feedback:
- SplitStringOnce is extracted out of the loop in IsReservedNameOnWindows
  to avoid redundant string splitting up to 23 times per path component.
- Added explanatory comments around the optimized prefix check.
- Passed IsReservedNameOnWindows by function pointer to std::ranges::any_of
  to prevent potential compiler/toolchain overload ambiguities.

Original change's description:
> storage.mojom.StrictRelativePath deserializes paths passed over
> Mojo IPC to FilesystemImpl. Previously, it only checked
> path.IsAbsolute() and path.ReferencesParent().
>
> On Windows, DOS reserved device names (such as CON, PRN, AUX,
> NUL, COM1-COM9, LPT1-LPT9, CLOCK$, CONIN$, CONOUT$) or names with
> trailing spaces/dots (which Win32 API canonicalization strips to
> the underlying device, e.g., "con ") are not considered absolute
> or parent-referencing by base::FilePath. When FilesystemImpl
> appends such a relative path to its root directory, Win32 APIs
> resolve the path to system DOS devices (\\.\\CON, \\.\\NUL, etc.),
> bypassing the directory sandbox.

Bug: 497203958, 545283003
Change-Id: Ieee521f20a134767a4a523d0b317cf58bf698ebb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8291418
Reviewed-by: Daniel Cheng <[email protected]>
Reviewed-by: Evan Stade <[email protected]>
Commit-Queue: S Ganesh <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1686747}
---

diff --git a/base/files/file_util.cc b/base/files/file_util.cc
index 6b1a6cb..fe72e5f 100644
--- a/base/files/file_util.cc
+++ b/base/files/file_util.cc
@@ -534,7 +534,6 @@
   return FilePath();
 }
 
-#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_POSIX) || BUILDFLAG(IS_FUCHSIA)
 bool IsReservedNameOnWindows(const base::FilePath::StringType& filename) {
   // This list is taken from the MSDN article "Naming a file"
   // http://msdn2.microsoft.com/en-us/library/aa365247(VS.85).aspx
@@ -549,26 +548,38 @@
       // shell.
       "desktop.ini",
       "thumbs.db",
+      // Windows console input/output devices. Unlike legacy DOS devices (e.g.
+      // CON), Windows does not strip extensions for CONIN$/CONOUT$.
+      "conin$",
+      "conout$",
   });
 
 #if BUILDFLAG(IS_WIN)
   std::string filename_lower = base::ToLowerASCII(base::WideToUTF8(filename));
-#elif BUILDFLAG(IS_POSIX) || BUILDFLAG(IS_FUCHSIA)
+#else
   std::string filename_lower = base::ToLowerASCII(filename);
 #endif
 
-  return std::ranges::any_of(kKnownDevices,
-                             [&filename_lower](std::string_view device) {
-                               if (filename_lower == device) {
-                                 return true;
-                               }
-                               auto parts =
-                                   SplitStringOnce(filename_lower, '.');
-                               return parts && parts->first == device;
-                             }) ||
-         kMagicNames.contains(filename_lower);
+  // On Windows, trailing spaces and dots are stripped by Win32 API path
+  // canonicalization (e.g., "con " or "con. " resolves to device "\\.\CON").
+  std::string_view trimmed_filename =
+      base::TrimString(filename_lower, " .", base::TRIM_TRAILING);
+
+  // Extract the part of the filename before the first dot to check against
+  // DOS device names (e.g. "CON.zip" -> "CON" and "CON.tar.gz" -> "CON").
+  // Doing this once here avoids redundant string splitting inside the loop.
+  std::string_view prefix = trimmed_filename;
+  if (auto parts = SplitStringOnce(trimmed_filename, '.')) {
+    prefix = parts->first;
+  }
+
+  return std::ranges::any_of(
+             kKnownDevices,
+             [prefix, trimmed_filename](std::string_view device) {
+               return trimmed_filename == device || prefix == device;
+             }) ||
+         kMagicNames.contains(trimmed_filename);
 }
-#endif  // BUILDFLAG(IS_WIN) || BUILDFLAG(IS_POSIX) || BUILDFLAG(IS_FUCHSIA)
 
 std::optional<FilePath> GetLatestTemporaryFileWithNamePrefix(
     const FilePath& dir,
diff --git a/base/files/file_util.h b/base/files/file_util.h
index 32e7134..e71ab46 100644
--- a/base/files/file_util.h
+++ b/base/files/file_util.h
@@ -811,7 +811,6 @@
 
 #endif
 
-#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_POSIX) || BUILDFLAG(IS_FUCHSIA)
 // Returns whether the specified file name is a reserved name on Windows.
 // This includes names like "com2.zip" (which correspond to devices) and
 // desktop.ini and thumbs.db which have special meaning to the Windows shell.
@@ -819,7 +818,6 @@
 // reserved on Windows.
 BASE_EXPORT bool IsReservedNameOnWindows(
     const base::FilePath::StringType& filename);
-#endif
 
 // Internal --------------------------------------------------------------------
 
diff --git a/base/files/file_util_unittest.cc b/base/files/file_util_unittest.cc
index 591de4bf..af06c8e 100644
--- a/base/files/file_util_unittest.cc
+++ b/base/files/file_util_unittest.cc
@@ -5631,7 +5631,6 @@
   ASSERT_EQ(second_file.GetLength(), 0);
 }
 
-#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_POSIX) || BUILDFLAG(IS_FUCHSIA)
 TEST_F(FileUtilTest, IsReservedNameOnWindows) {
   static constexpr auto kAllowedBasenames =
       std::to_array<const base::FilePath::CharType*>({
@@ -5640,15 +5639,24 @@
           FILE_PATH_LITERAL("a b.txt"),
           FILE_PATH_LITERAL("a-b.txt"),
           FILE_PATH_LITERAL("My Computer"),
+          FILE_PATH_LITERAL("conin$.txt"),
+          FILE_PATH_LITERAL("conout$.log"),
       });
 
   static constexpr auto kDisallowedBasenames =
       std::to_array<const base::FilePath::CharType*>({
           FILE_PATH_LITERAL("con"),
+          FILE_PATH_LITERAL("con "),
+          FILE_PATH_LITERAL("con. "),
           FILE_PATH_LITERAL("con.zip"),
+          FILE_PATH_LITERAL("conin$"),
+          FILE_PATH_LITERAL("conin$ "),
+          FILE_PATH_LITERAL("conout$"),
+          FILE_PATH_LITERAL("aux . "),
           FILE_PATH_LITERAL("NUL"),
           FILE_PATH_LITERAL("NUL.zip"),
           FILE_PATH_LITERAL("desktop.ini"),
+          FILE_PATH_LITERAL("desktop.ini "),
       });
 
   for (const base::FilePath::CharType* basename : kAllowedBasenames) {
@@ -5659,7 +5667,6 @@
     EXPECT_TRUE(IsReservedNameOnWindows(basename)) << basename;
   }
 }
-#endif  // BUILDFLAG(IS_WIN) || BUILDFLAG(IS_POSIX) || BUILDFLAG(IS_FUCHSIA)
 
 }  // namespace
 
diff --git a/components/services/storage/public/cpp/filesystem/BUILD.gn b/components/services/storage/public/cpp/filesystem/BUILD.gn
index 84608110..4aec4e5 100644
--- a/components/services/storage/public/cpp/filesystem/BUILD.gn
+++ b/components/services/storage/public/cpp/filesystem/BUILD.gn
@@ -41,12 +41,17 @@
 
 source_set("tests") {
   testonly = true
-  sources = [ "filesystem_proxy_unittest.cc" ]
+  sources = [
+    "filesystem_proxy_unittest.cc",
+    "strict_relative_path_mojom_traits_unittest.cc",
+  ]
   deps = [
     ":filesystem",
+    ":typemap_traits",
     "//base",
     "//base/test:test_support",
     "//components/services/storage/public/mojom/filesystem",
+    "//mojo/public/cpp/test_support:test_utils",
     "//testing/gmock",
     "//testing/gtest",
   ]
diff --git a/components/services/storage/public/cpp/filesystem/strict_relative_path_mojom_traits.cc b/components/services/storage/public/cpp/filesystem/strict_relative_path_mojom_traits.cc
index 6b41b94e..d264d9e 100644
--- a/components/services/storage/public/cpp/filesystem/strict_relative_path_mojom_traits.cc
+++ b/components/services/storage/public/cpp/filesystem/strict_relative_path_mojom_traits.cc

diff --git a/base/files/file_util_unittest.cc b/base/files/file_util_unittest.cc
index 591de4bf..af06c8e 100644
--- a/base/files/file_util_unittest.cc
+++ b/base/files/file_util_unittest.cc
@@ -5631,7 +5631,6 @@
   ASSERT_EQ(second_file.GetLength(), 0);
 }
 
-#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_POSIX) || BUILDFLAG(IS_FUCHSIA)
 TEST_F(FileUtilTest, IsReservedNameOnWindows) {
   static constexpr auto kAllowedBasenames =
       std::to_array<const base::FilePath::CharType*>({
@@ -5640,15 +5639,24 @@
           FILE_PATH_LITERAL("a b.txt"),
           FILE_PATH_LITERAL("a-b.txt"),
           FILE_PATH_LITERAL("My Computer"),
+          FILE_PATH_LITERAL("conin$.txt"),
+          FILE_PATH_LITERAL("conout$.log"),
       });
 
   static constexpr auto kDisallowedBasenames =
       std::to_array<const base::FilePath::CharType*>({
           FILE_PATH_LITERAL("con"),
+          FILE_PATH_LITERAL("con "),
+          FILE_PATH_LITERAL("con. "),
           FILE_PATH_LITERAL("con.zip"),
+          FILE_PATH_LITERAL("conin$"),
+          FILE_PATH_LITERAL("conin$ "),
+          FILE_PATH_LITERAL("conout$"),
+          FILE_PATH_LITERAL("aux . "),
           FILE_PATH_LITERAL("NUL"),
           FILE_PATH_LITERAL("NUL.zip"),
           FILE_PATH_LITERAL("desktop.ini"),
+          FILE_PATH_LITERAL("desktop.ini "),
       });
 
   for (const base::FilePath::CharType* basename : kAllowedBasenames) {
@@ -5659,7 +5667,6 @@
     EXPECT_TRUE(IsReservedNameOnWindows(basename)) << basename;
   }
 }
-#endif  // BUILDFLAG(IS_WIN) || BUILDFLAG(IS_POSIX) || BUILDFLAG(IS_FUCHSIA)
 
 }  // namespace
diff --git a/components/services/storage/public/cpp/filesystem/strict_relative_path_mojom_traits_unittest.cc b/components/services/storage/public/cpp/filesystem/strict_relative_path_mojom_traits_unittest.cc
new file mode 100644
index 0000000..7ee73c3
--- /dev/null
+++ b/components/services/storage/public/cpp/filesystem/strict_relative_path_mojom_traits_unittest.cc
@@ -0,0 +1,87 @@
+// Copyright 2026 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "components/services/storage/public/cpp/filesystem/strict_relative_path_mojom_traits.h"
+
+#include "base/files/file_path.h"
+#include "build/build_config.h"
+#include "components/services/storage/public/mojom/filesystem/directory.mojom.h"
+#include "mojo/public/cpp/test_support/test_utils.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace storage {
+namespace {
+
+TEST(StrictRelativePathTraitsTest, ValidRelativePaths) {
+  const base::FilePath test_paths[] = {
+      base::FilePath(FILE_PATH_LITERAL("file.txt")),
+      base::FilePath(FILE_PATH_LITERAL("dir/file.txt")),
+      base::FilePath(FILE_PATH_LITERAL("dir/subdir/file.txt")),
+  };
+
+  for (const auto& original : test_paths) {
+    base::FilePath deserialized;
+    EXPECT_TRUE(mojo::test::SerializeAndDeserialize<mojom::StrictRelativePath>(
+        original, deserialized));
+    EXPECT_EQ(original, deserialized);
+  }
+}
+
+TEST(StrictRelativePathTraitsTest, RejectsInvalidPaths) {
+  const base::FilePath test_paths[] = {
+      base::FilePath(FILE_PATH_LITERAL("../file.txt")),
+      base::FilePath(FILE_PATH_LITERAL("dir/../file.txt")),
+  };
+
+  for (const auto& original : test_paths) {
+    base::FilePath deserialized;
+    EXPECT_FALSE(mojo::test::SerializeAndDeserialize<mojom::StrictRelativePath>(
+        original, deserialized));
+  }
+}
+
+TEST(StrictRelativePathTraitsTest, RejectsReservedWindowsNames) {
+  const base::FilePath test_paths[] = {
+      base::FilePath(FILE_PATH_LITERAL("con")),
+      base::FilePath(FILE_PATH_LITERAL("con ")),
+      base::FilePath(FILE_PATH_LITERAL("con. ")),
+      base::FilePath(FILE_PATH_LITERAL("con.txt")),
+      base::FilePath(FILE_PATH_LITERAL("conin$")),
+      base::FilePath(FILE_PATH_LITERAL("conin$ ")),
+      base::FilePath(FILE_PATH_LITERAL("conout$")),
+      base::FilePath(FILE_PATH_LITERAL("nul")),
+      base::FilePath(FILE_PATH_LITERAL("dir1/nul")),
+      base::FilePath(FILE_PATH_LITERAL("dir1/aux/file.txt")),
+      base::FilePath(FILE_PATH_LITERAL("dir1/aux . /file.txt")),
+  };
+
+  for (const auto& original : test_paths) {
+    base::FilePath deserialized;
+#if BUILDFLAG(IS_WIN)
+    EXPECT_FALSE(mojo::test::SerializeAndDeserialize<mojom::StrictRelativePath>(
+        original, deserialized));
+#else
+    EXPECT_TRUE(mojo::test::SerializeAndDeserialize<mojom::StrictRelativePath>(
+        original, deserialized));
+    EXPECT_EQ(original, deserialized);
+#endif
+  }
+}
+
+TEST(StrictRelativePathTraitsTest, AllowsHarmlessExtensionsOnSpecialDevices) {
+  const base::FilePath test_paths[] = {
+      base::FilePath(FILE_PATH_LITERAL("conin$.txt")),
+      base::FilePath(FILE_PATH_LITERAL("conout$.log")),
+  };
+
+  for (const auto& original : test_paths) {
+    base::FilePath deserialized;
+    EXPECT_TRUE(mojo::test::SerializeAndDeserialize<mojom::StrictRelativePath>(
+        original, deserialized));
+    EXPECT_EQ(original, deserialized);
+  }
+}
+
+}  // namespace
+}  // namespace storage