From 64c8e2a1cedafdbcc750abc423ad9b389537c223 Mon Sep 17 00:00:00 2001
From: Ming-Ying Chung <[email protected]>
Date: Thu, 17 Sep 2026 20:48:27 -0700
Subject: [PATCH] [FSA] Block macOS firmlink alias paths to sensitive directories

On macOS 10.15+, the system storage layout splits into a read-only
System volume mounted at "/" and a read-write Data volume mounted at
"/System/Volumes/Data". APFS firmlinks map directories such as /Users,
/Applications, and /Library between the two volumes without modifying
standard POSIX realpath representations.

Previously, ChromeFileSystemAccessPermissionContext matched candidate
file and directory paths lexically against canonical blocklist rules
registered under the "/" root namespace. When a target path was
referenced or resolved via "/System/Volumes/Data/...", component-wise
path matching diverged at the leading path component, failing open and
bypassing sensitive directory protections for targets like ~/.ssh,
~/Library, and Chrome User Data.

To prevent blocklist bypasses while preserving legitimate user access,
this change introduces MaybeStripMacDataVolumePrefix() on macOS to strip
the "/System/Volumes/Data" prefix and fold paths onto the "/" namespace
prior to ancestor evaluation in ShouldBlockAccessToPath(). Additionally,
it registers an explicit kBlockAllChildren rule for /System/Volumes in
GenerateBlockPaths() to block direct access to internal system container
volumes.

Bug: 562043997
Change-Id: Id80dfecafad651d28b691e10d960bc6e3ac5a763
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8410087
Reviewed-by: Mingyu Lei <[email protected]>
Commit-Queue: Ming-Ying Chung <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1700449}
---
diff --git a/chrome/browser/file_system_access/chrome_file_system_access_permission_context.cc b/chrome/browser/file_system_access/chrome_file_system_access_permission_context.cc
index 1c82e07..7d834cc 100644
--- a/chrome/browser/file_system_access/chrome_file_system_access_permission_context.cc
+++ b/chrome/browser/file_system_access/chrome_file_system_access_permission_context.cc
@@ -341,6 +341,32 @@
 }
 #endif
 
+#if BUILDFLAG(IS_MAC)
+// Returns a file path with the "/System/Volumes/Data" firmlink prefix stripped,
+// mapping paths on the macOS Data volume back to the root "/" namespace.
+//
+// On macOS 10.15+, storage is split into a read-only System volume mounted at
+// "/" and a read-write Data volume mounted at "/System/Volumes/Data". APFS
+// firmlinks map system directories (e.g., "/Users", "/Applications",
+// "/Library") between the two volumes without modifying standard POSIX
+// `realpath` representations. Stripping this prefix allows blocklist rules
+// defined against root-relative paths to match targets accessed through the
+// Data volume firmlink path.
+base::FilePath MaybeStripMacDataVolumePrefix(const base::FilePath& path) {
+  static constexpr base::FilePath::CharType kDataVolumePrefix[] =
+      FILE_PATH_LITERAL("/System/Volumes/Data");
+  base::FilePath data_volume_prefix(kDataVolumePrefix);
+  if (path == data_volume_prefix) {
+    return base::FilePath(FILE_PATH_LITERAL("/"));
+  }
+  base::FilePath stripped_path(FILE_PATH_LITERAL("/"));
+  if (data_volume_prefix.AppendRelativePath(path, &stripped_path)) {
+    return stripped_path;
+  }
+  return path;
+}
+#endif
+
 // A wrapper around `base::NormalizeFilePath` that returns its result instead of
 // using an out parameter.
 base::FilePath NormalizeFilePath(const base::FilePath& path) {
@@ -366,6 +392,20 @@
   return normalized_path;
 }
 
+// Normalizes `path` for sensitive directory blocklist checking. Resolves
+// symbolic links and canonicalizes the path when `should_normalize_file_path`
+// is true. On macOS, additionally strips the "/System/Volumes/Data" firmlink
+// prefix so paths on the Data volume map onto the root "/" blocklist namespace.
+base::FilePath NormalizeFilePathForBlocklist(const base::FilePath& path,
+                                             bool should_normalize_file_path) {
+  base::FilePath normalized_path =
+      should_normalize_file_path ? NormalizeFilePath(path) : path;
+#if BUILDFLAG(IS_MAC)
+  normalized_path = MaybeStripMacDataVolumePrefix(normalized_path);
+#endif
+  return normalized_path;
+}
+
 using BlockType = ChromeFileSystemAccessPermissionContext::BlockType;
 
 std::unique_ptr<ChromeFileSystemAccessPermissionContext::BlockPathRules>
@@ -430,6 +470,10 @@
 #endif
 #if BUILDFLAG(IS_MAC)
       // Similar Mac specific blocks.
+      // Block access to /System/Volumes (internal system and container
+      // volumes).
+      BlockPath::CreateAbsolute(FILE_PATH_LITERAL("/System/Volumes"),
+                                BlockType::kBlockAllChildren),
       BlockPath::CreateRelative(base::DIR_APP_DATA,
                                 BlockType::kBlockAllChildren),
       // Block access to the current bundle directory.
@@ -546,7 +590,7 @@
     }
 
     block_path_rules->block_path_rules_.emplace_back(
-        should_normalize_file_path ? NormalizeFilePath(path) : path,
+        NormalizeFilePathForBlocklist(path, should_normalize_file_path),
         blocked_path.block_type);
   }
 
@@ -572,12 +616,12 @@
   DCHECK(!path.empty());
   DCHECK(path.IsAbsolute());
 
-  if (should_normalize_file_path) {
-    path = NormalizeFilePath(path);
-    profile_path = NormalizeFilePath(profile_path);
-    for (auto& rule : extra_rules) {
-      rule.path = NormalizeFilePath(rule.path);
-    }
+  path = NormalizeFilePathForBlocklist(path, should_normalize_file_path);
+  profile_path =
+      NormalizeFilePathForBlocklist(profile_path, should_normalize_file_path);
+  for (auto& rule : extra_rules) {
+    rule.path =
+        NormalizeFilePathForBlocklist(rule.path, should_normalize_file_path);
   }
 
 #if BUILDFLAG(IS_WIN)
diff --git a/chrome/browser/file_system_access/chrome_file_system_access_permission_context_unittest.cc b/chrome/browser/file_system_access/chrome_file_system_access_permission_context_unittest.cc
index e7676f9..0c8a2d7 100644
--- a/chrome/browser/file_system_access/chrome_file_system_access_permission_context_unittest.cc
+++ b/chrome/browser/file_system_access/chrome_file_system_access_permission_context_unittest.cc
@@ -974,6 +974,113 @@
                 HandleType::kDirectory, UserAction::kOpen),
             SensitiveDirectoryResult::kAbort);
 }
+
+// Verifies that sensitive directories and system paths accessed through APFS
+// firmlink aliases under `/System/Volumes/Data` or internal system volumes
+// under `/System/Volumes` are blocked.
+//
+// On macOS 10.15+, user home folders, `/Applications`, and `/Library` reside on
+// the read-write Data volume mounted at `/System/Volumes/Data`. This test
+// verifies that referencing sensitive targets through the
+// `/System/Volumes/Data` alias spelling fails sensitive entry access checks and
+// returns `kAbort`.
+TEST_F(ChromeFileSystemAccessPermissionContextTest,
+       ConfirmSensitiveEntryAccess_BlockFirmlinkedSystemAndUserPaths) {
+  base::FilePath home_dir = temp_dir_.GetPath().AppendASCII("home");
+  ScopedHomeDirOverride home_override = OverrideHomeDir(home_dir);
+  ResetBlockPath();
+
+  const base::FilePath kDataVolumePrefix(
+      FILE_PATH_LITERAL("/System/Volumes/Data"));
+
+  // Verify that `~/.ssh` and its private keys are blocked when accessed via the
+  // data volume alias.
+  base::FilePath ssh_under_data(FILE_PATH_LITERAL("/System/Volumes/Data") +
+                                home_dir.AppendASCII(".ssh").value());
+  EXPECT_EQ(ConfirmSensitiveEntryAccessSync(
+                permission_context(), PathInfo(ssh_under_data),
+                HandleType::kDirectory, UserAction::kOpen),
+            SensitiveDirectoryResult::kAbort);
+  EXPECT_EQ(
+      ConfirmSensitiveEntryAccessSync(
+          permission_context(), PathInfo(ssh_under_data.AppendASCII("id_rsa")),
+          HandleType::kFile, UserAction::kOpen),
+      SensitiveDirectoryResult::kAbort);
+
+  // Verify that `~/Library` and Chrome user data directories are blocked when
+  // accessed via the data volume alias.
+  base::FilePath library_under_data(FILE_PATH_LITERAL("/System/Volumes/Data") +
+                                    home_dir.AppendASCII("Library").value());
+  EXPECT_EQ(ConfirmSensitiveEntryAccessSync(
+                permission_context(), PathInfo(library_under_data),
+                HandleType::kDirectory, UserAction::kOpen),
+            SensitiveDirectoryResult::kAbort);
+
+  // Verify that the `/System/Volumes/Data` mount point root itself is blocked.
+  EXPECT_EQ(ConfirmSensitiveEntryAccessSync(
+                permission_context(), PathInfo(kDataVolumePrefix),
+                HandleType::kDirectory, UserAction::kOpen),
+            SensitiveDirectoryResult::kAbort);
+
+  // Verify that internal system volumes located under `/System/Volumes` such as
+  // the dynamic virtual memory swap volume and preboot volume are blocked.
+  EXPECT_EQ(
+      ConfirmSensitiveEntryAccessSync(
+          permission_context(), PathInfo(FILE_PATH_LITERAL("/System/Volumes")),
+          HandleType::kDirectory, UserAction::kOpen),
+      SensitiveDirectoryResult::kAbort);
+  EXPECT_EQ(ConfirmSensitiveEntryAccessSync(
+                permission_context(),
+                PathInfo(FILE_PATH_LITERAL("/System/Volumes/VM")),
+                HandleType::kDirectory, UserAction::kOpen),
+            SensitiveDirectoryResult::kAbort);
+  EXPECT_EQ(ConfirmSensitiveEntryAccessSync(
+                permission_context(),
+                PathInfo(FILE_PATH_LITERAL("/System/Volumes/Preboot")),
+                HandleType::kDirectory, UserAction::kOpen),
+            SensitiveDirectoryResult::kAbort);
+}
+
+// Verifies that allowed subdirectories within user home folders (such as
+// `~/Downloads`, `~/Library/CloudStorage`, and `~/Library/Containers`) remain
+// accessible when referenced through `/System/Volumes/Data`.
+//
+// Normalization must preserve descendant exceptions defined under
+// `kDontBlockChildren` so legitimate user workflows succeed regardless of
+// whether the path is referenced via `/` or `/System/Volumes/Data`.
+TEST_F(ChromeFileSystemAccessPermissionContextTest,
+       ConfirmSensitiveEntryAccess_AllowFirmlinkedUserSubdirectories) {
+  base::FilePath home_dir = temp_dir_.GetPath().AppendASCII("home");
+  ScopedHomeDirOverride home_override = OverrideHomeDir(home_dir);
+  ResetBlockPath();
+
+  // Child files and subdirectories inside `~/Downloads` should be permitted.
+  base::FilePath data_downloads(FILE_PATH_LITERAL("/System/Volumes/Data") +
+                                home_dir.AppendASCII("Downloads").value());
+  EXPECT_EQ(ConfirmSensitiveEntryAccessSync(
+                permission_context(),
+                PathInfo(data_downloads.AppendASCII("report.pdf")),
+                HandleType::kFile, UserAction::kOpen),
+            SensitiveDirectoryResult::kAllowed);
+  EXPECT_EQ(ConfirmSensitiveEntryAccessSync(
+                permission_context(),
+                PathInfo(data_downloads.AppendASCII("subfolder")),
+                HandleType::kDirectory, UserAction::kOpen),
+            SensitiveDirectoryResult::kAllowed);
+
+  // External drives mounted under `/Volumes` should remain accessible.
+  base::FilePath external_volume(
+      FILE_PATH_LITERAL("/Volumes/ExternalUSB/Projects/my_code"));
+  EXPECT_EQ(ConfirmSensitiveEntryAccessSync(
+                permission_context(), PathInfo(external_volume),
+                HandleType::kDirectory, UserAction::kOpen),
+            SensitiveDirectoryResult::kAllowed);
+  EXPECT_EQ(ConfirmSensitiveEntryAccessSync(
+                permission_context(),
+                PathInfo(external_volume.AppendASCII("main.rs")),
+                HandleType::kFile, UserAction::kOpen),
+            SensitiveDirectoryResult::kAllowed);
+}
 #endif  // BUILDFLAG(IS_MAC)
 
 #if BUILDFLAG(IS_WIN)
@@ -1502,6 +1609,40 @@
 }
 #endif  // BUILDFLAG(IS_WIN)
 
+#if BUILDFLAG(IS_MAC)
+// Verifies that symbolic links resolving into sensitive paths on the data
+// volume (`/System/Volumes/Data`) are caught and blocked by symlink checks.
+TEST_F(ChromeFileSystemAccessPermissionContextSymbolicLinkCheckTest,
+       ConfirmSensitiveEntryAccess_ResolveSymbolicLinkToDataVolume) {
+  // Create a real ~/.ssh directory in the temporary home folder.
+  base::FilePath home_dir = temp_dir_.GetPath().AppendASCII("home");
+  base::FilePath ssh_dir = home_dir.AppendASCII(".ssh");
+  ASSERT_TRUE(base::CreateDirectory(ssh_dir));
+
+  ScopedHomeDirOverride home_override = OverrideHomeDir(home_dir);
+  ResetBlockPath();
+
+  // On macOS, `temp_dir_` (/private/var/...) physically resides on the Data
+  // volume mounted at `/System/Volumes/Data`.
+  base::FilePath absolute_ssh_dir = base::MakeAbsoluteFilePath(ssh_dir);
+  ASSERT_FALSE(absolute_ssh_dir.empty());
+
+  base::FilePath target_data_ssh(FILE_PATH_LITERAL("/System/Volumes/Data") +
+                                 absolute_ssh_dir.value());
+  ASSERT_TRUE(base::PathExists(target_data_ssh));
+
+  base::FilePath symlink_path =
+      temp_dir_.GetPath().AppendASCII("symlink_to_data_ssh");
+  ASSERT_EQ(CreateSymbolicLinkForTesting(target_data_ssh, symlink_path),
+            CreateSymbolicLinkResult::kSucceeded);
+
+  EXPECT_EQ(ConfirmSensitiveEntryAccessSync(
+                permission_context(), PathInfo(symlink_path),
+                HandleType::kDirectory, UserAction::kOpen),
+            SensitiveDirectoryResult::kAbort);
+}
+#endif  // BUILDFLAG(IS_MAC)
+
 TEST_F(ChromeFileSystemAccessPermissionContextTest,
        ConfirmSensitiveEntryAccess_DangerousFile) {
   base::FilePath home_dir = temp_dir_.GetPath().AppendASCII("home");

diff --git a/chrome/browser/file_system_access/chrome_file_system_access_permission_context_unittest.cc b/chrome/browser/file_system_access/chrome_file_system_access_permission_context_unittest.cc
index e7676f9..0c8a2d7 100644
--- a/chrome/browser/file_system_access/chrome_file_system_access_permission_context_unittest.cc
+++ b/chrome/browser/file_system_access/chrome_file_system_access_permission_context_unittest.cc
@@ -974,6 +974,113 @@
                 HandleType::kDirectory, UserAction::kOpen),
             SensitiveDirectoryResult::kAbort);
 }
+
+// Verifies that sensitive directories and system paths accessed through APFS
+// firmlink aliases under `/System/Volumes/Data` or internal system volumes
+// under `/System/Volumes` are blocked.
+//
+// On macOS 10.15+, user home folders, `/Applications`, and `/Library` reside on
+// the read-write Data volume mounted at `/System/Volumes/Data`. This test
+// verifies that referencing sensitive targets through the
+// `/System/Volumes/Data` alias spelling fails sensitive entry access checks and
+// returns `kAbort`.
+TEST_F(ChromeFileSystemAccessPermissionContextTest,
+       ConfirmSensitiveEntryAccess_BlockFirmlinkedSystemAndUserPaths) {
+  base::FilePath home_dir = temp_dir_.GetPath().AppendASCII("home");
+  ScopedHomeDirOverride home_override = OverrideHomeDir(home_dir);
+  ResetBlockPath();
+
+  const base::FilePath kDataVolumePrefix(
+      FILE_PATH_LITERAL("/System/Volumes/Data"));
+
+  // Verify that `~/.ssh` and its private keys are blocked when accessed via the
+  // data volume alias.
+  base::FilePath ssh_under_data(FILE_PATH_LITERAL("/System/Volumes/Data") +
+                                home_dir.AppendASCII(".ssh").value());
+  EXPECT_EQ(ConfirmSensitiveEntryAccessSync(
+                permission_context(), PathInfo(ssh_under_data),
+                HandleType::kDirectory, UserAction::kOpen),
+            SensitiveDirectoryResult::kAbort);
+  EXPECT_EQ(
+      ConfirmSensitiveEntryAccessSync(
+          permission_context(), PathInfo(ssh_under_data.AppendASCII("id_rsa")),
+          HandleType::kFile, UserAction::kOpen),
+      SensitiveDirectoryResult::kAbort);
+
+  // Verify that `~/Library` and Chrome user data directories are blocked when
+  // accessed via the data volume alias.
+  base::FilePath library_under_data(FILE_PATH_LITERAL("/System/Volumes/Data") +
+                                    home_dir.AppendASCII("Library").value());
+  EXPECT_EQ(ConfirmSensitiveEntryAccessSync(
+                permission_context(), PathInfo(library_under_data),
+                HandleType::kDirectory, UserAction::kOpen),
+            SensitiveDirectoryResult::kAbort);
+
+  // Verify that the `/System/Volumes/Data` mount point root itself is blocked.
+  EXPECT_EQ(ConfirmSensitiveEntryAccessSync(
+                permission_context(), PathInfo(kDataVolumePrefix),
+                HandleType::kDirectory, UserAction::kOpen),
+            SensitiveDirectoryResult::kAbort);
+
+  // Verify that internal system volumes located under `/System/Volumes` such as
+  // the dynamic virtual memory swap volume and preboot volume are blocked.
+  EXPECT_EQ(
+      ConfirmSensitiveEntryAccessSync(
+          permission_context(), PathInfo(FILE_PATH_LITERAL("/System/Volumes")),
+          HandleType::kDirectory, UserAction::kOpen),
+      SensitiveDirectoryResult::kAbort);
+  EXPECT_EQ(ConfirmSensitiveEntryAccessSync(
+                permission_context(),
+                PathInfo(FILE_PATH_LITERAL("/System/Volumes/VM")),
+                HandleType::kDirectory, UserAction::kOpen),
+            SensitiveDirectoryResult::kAbort);
+  EXPECT_EQ(ConfirmSensitiveEntryAccessSync(
+                permission_context(),
+                PathInfo(FILE_PATH_LITERAL("/System/Volumes/Preboot")),
+                HandleType::kDirectory, UserAction::kOpen),
+            SensitiveDirectoryResult::kAbort);
+}
+
+// Verifies that allowed subdirectories within user home folders (such as
+// `~/Downloads`, `~/Library/CloudStorage`, and `~/Library/Containers`) remain
+// accessible when referenced through `/System/Volumes/Data`.
+//
+// Normalization must preserve descendant exceptions defined under
+// `kDontBlockChildren` so legitimate user workflows succeed regardless of
+// whether the path is referenced via `/` or `/System/Volumes/Data`.
+TEST_F(ChromeFileSystemAccessPermissionContextTest,
+       ConfirmSensitiveEntryAccess_AllowFirmlinkedUserSubdirectories) {
+  base::FilePath home_dir = temp_dir_.GetPath().AppendASCII("home");
+  ScopedHomeDirOverride home_override = OverrideHomeDir(home_dir);
+  ResetBlockPath();
+
+  // Child files and subdirectories inside `~/Downloads` should be permitted.
+  base::FilePath data_downloads(FILE_PATH_LITERAL("/System/Volumes/Data") +
+                                home_dir.AppendASCII("Downloads").value());
+  EXPECT_EQ(ConfirmSensitiveEntryAccessSync(
+                permission_context(),
+                PathInfo(data_downloads.AppendASCII("report.pdf")),
+                HandleType::kFile, UserAction::kOpen),
+            SensitiveDirectoryResult::kAllowed);
+  EXPECT_EQ(ConfirmSensitiveEntryAccessSync(
+                permission_context(),
+                PathInfo(data_downloads.AppendASCII("subfolder")),
+                HandleType::kDirectory, UserAction::kOpen),
+            SensitiveDirectoryResult::kAllowed);
+
+  // External drives mounted under `/Volumes` should remain accessible.
+  base::FilePath external_volume(
+      FILE_PATH_LITERAL("/Volumes/ExternalUSB/Projects/my_code"));
+  EXPECT_EQ(ConfirmSensitiveEntryAccessSync(
+                permission_context(), PathInfo(external_volume),
+                HandleType::kDirectory, UserAction::kOpen),
+            SensitiveDirectoryResult::kAllowed);
+  EXPECT_EQ(ConfirmSensitiveEntryAccessSync(
+                permission_context(),
+                PathInfo(external_volume.AppendASCII("main.rs")),
+                HandleType::kFile, UserAction::kOpen),
+            SensitiveDirectoryResult::kAllowed);
+}
 #endif  // BUILDFLAG(IS_MAC)
 
 #if BUILDFLAG(IS_WIN)
@@ -1502,6 +1609,40 @@
 }
 #endif  // BUILDFLAG(IS_WIN)
 
+#if BUILDFLAG(IS_MAC)
+// Verifies that symbolic links resolving into sensitive paths on the data
+// volume (`/System/Volumes/Data`) are caught and blocked by symlink checks.
+TEST_F(ChromeFileSystemAccessPermissionContextSymbolicLinkCheckTest,
+       ConfirmSensitiveEntryAccess_ResolveSymbolicLinkToDataVolume) {
+  // Create a real ~/.ssh directory in the temporary home folder.
+  base::FilePath home_dir = temp_dir_.GetPath().AppendASCII("home");
+  base::FilePath ssh_dir = home_dir.AppendASCII(".ssh");
+  ASSERT_TRUE(base::CreateDirectory(ssh_dir));
+
+  ScopedHomeDirOverride home_override = OverrideHomeDir(home_dir);
+  ResetBlockPath();
+
+  // On macOS, `temp_dir_` (/private/var/...) physically resides on the Data
+  // volume mounted at `/System/Volumes/Data`.
+  base::FilePath absolute_ssh_dir = base::MakeAbsoluteFilePath(ssh_dir);
+  ASSERT_FALSE(absolute_ssh_dir.empty());
+
+  base::FilePath target_data_ssh(FILE_PATH_LITERAL("/System/Volumes/Data") +
+                                 absolute_ssh_dir.value());
+  ASSERT_TRUE(base::PathExists(target_data_ssh));
+
+  base::FilePath symlink_path =
+      temp_dir_.GetPath().AppendASCII("symlink_to_data_ssh");
+  ASSERT_EQ(CreateSymbolicLinkForTesting(target_data_ssh, symlink_path),
+            CreateSymbolicLinkResult::kSucceeded);
+
+  EXPECT_EQ(ConfirmSensitiveEntryAccessSync(
+                permission_context(), PathInfo(symlink_path),
+                HandleType::kDirectory, UserAction::kOpen),
+            SensitiveDirectoryResult::kAbort);
+}
+#endif  // BUILDFLAG(IS_MAC)
+
 TEST_F(ChromeFileSystemAccessPermissionContextTest,
        ConfirmSensitiveEntryAccess_DangerousFile) {
   base::FilePath home_dir = temp_dir_.GetPath().AppendASCII("home");