diff --git a/patches/chromium/refactor_expose_file_system_access_blocklist.patch b/patches/chromium/refactor_expose_file_system_access_blocklist.patch index 2aa028bf91..dd2e4c9c14 100644 --- a/patches/chromium/refactor_expose_file_system_access_blocklist.patch +++ b/patches/chromium/refactor_expose_file_system_access_blocklist.patch @@ -7,15 +7,20 @@ This CL exposes the file system access blocklist publicly so that we can leverag it in Electron and prevent drift from Chrome's blocklist. We should look for a way to upstream this change to Chrome. -NormalizeFilePathForBlocklist() and GenerateBlockPaths() are declared as static -members in the header, and their definitions (along with the NormalizeFilePath() -and MaybeStripMacDataVolumePrefix() helpers they use) are moved verbatim out of +NormalizeFilePathForBlocklist(), GenerateBlockPaths() and +ShouldBlockAccessToPath() are declared as static members in the header, and +their definitions (along with the NormalizeFilePath() and +MaybeStripMacDataVolumePrefix() helpers, and the Windows-only +ContainsInvalidDNSCharacter(), IsLocalUNCServer() and MaybeIsLocalUNCPath() +helpers they use) are moved verbatim out of chrome_file_system_access_permission_context.cc into a new -chrome_file_system_access_blocklist.cc, which Electron compiles. Upstream changes -to the blocklist therefore conflict on the hunk that removes them from -chrome_file_system_access_permission_context.cc instead of being silently -dropped; when resolving such a conflict, carry the change over into -chrome_file_system_access_blocklist.cc. +chrome_file_system_access_blocklist.cc, which Electron compiles. Electron +therefore uses Chrome's own path matcher as well as its rules, including suffix +rules such as .git/hooks, kBlockWrite handling and the local UNC path checks. +Upstream changes to the blocklist or the matcher conflict on the hunks that +remove them from chrome_file_system_access_permission_context.cc instead of +being silently dropped; when resolving such a conflict, carry the change over +into chrome_file_system_access_blocklist.cc. As of 158.0.8118.0, chrome_file_system_access_blocklist.cc also carries the upstream kBlockPaths additions from that version (~/.config on all platforms, @@ -36,10 +41,10 @@ index e5146cc3ed80f558fe729e76158463a4d6fed518..cf3a3a321286972783b8bb3e85fc50b6 "file_system_access_permission_context_factory.cc", diff --git a/chrome/browser/file_system_access/chrome_file_system_access_blocklist.cc b/chrome/browser/file_system_access/chrome_file_system_access_blocklist.cc new file mode 100644 -index 0000000000000000000000000000000000000000..7cec4f2dd24f5c376293b62aad51994c7601814c +index 0000000000000000000000000000000000000000..c26eb34183aa46fd2d5886df768ab909e7484fde --- /dev/null +++ b/chrome/browser/file_system_access/chrome_file_system_access_blocklist.cc -@@ -0,0 +1,330 @@ +@@ -0,0 +1,567 @@ +// 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. @@ -50,13 +55,16 @@ index 0000000000000000000000000000000000000000..7cec4f2dd24f5c376293b62aad51994c +// chrome_file_system_access_permission_context.cc can still use Chrome's +// blocklist rules. + ++#include +#include ++#include + +#include "base/base_paths.h" +#include "base/check.h" +#include "base/check_op.h" +#include "base/files/file_path.h" +#include "base/files/file_util.h" ++#include "base/logging.h" +#include "base/path_service.h" +#include "build/build_config.h" +#include "chrome/browser/file_system_access/chrome_file_system_access_permission_context.h" @@ -64,6 +72,99 @@ index 0000000000000000000000000000000000000000..7cec4f2dd24f5c376293b62aad51994c + +namespace { + ++#if BUILDFLAG(IS_WIN) ++bool ContainsInvalidDNSCharacter(base::FilePath::StringType hostname) { ++ for (base::FilePath::CharType c : hostname) { ++ if (!((c >= L'A' && c <= L'Z') || (c >= L'a' && c <= L'z') || ++ (c >= L'0' && c <= L'9') || (c == L'.') || (c == L'-') || ++ (c == L'_'))) { ++ return true; ++ } ++ } ++ return false; ++} ++ ++bool IsLocalUNCServer(base::FilePath::StringViewType server) { ++ if (server.size() > 1 && server.back() == FILE_PATH_LITERAL('.')) { ++ server.remove_suffix(1); ++ } ++ static constexpr base::FilePath::StringViewType kLocalServers[] = { ++ FILE_PATH_LITERAL("localhost"), ++ FILE_PATH_LITERAL("wsl.localhost"), ++ // Hypervisor host-share redirectors (Parallels, VMware, VirtualBox). ++ FILE_PATH_LITERAL("mac"), ++ FILE_PATH_LITERAL("psf"), ++ FILE_PATH_LITERAL("vmware-host"), ++ FILE_PATH_LITERAL(".host"), ++ FILE_PATH_LITERAL("vboxsvr"), ++ FILE_PATH_LITERAL("vboxsrv"), ++ }; ++ for (const auto& candidate : kLocalServers) { ++ if (base::FilePath::CompareEqualIgnoreCase(server, candidate)) { ++ return true; ++ } ++ } ++ return false; ++} ++ ++// Returns true if the path is a Universal Naming Convention (UNC) path pointing ++// to a local system path, device namespace, WSL loopback redirector, or ++// hypervisor host-share namespace. ++bool MaybeIsLocalUNCPath(const base::FilePath& path) { ++ if (!path.IsNetwork()) { ++ return false; ++ } ++ ++ const std::vector components = ++ path.GetComponents(); ++ ++ // Check for server name that could represent a local or host system. We only ++ // check for a very short list, as it is impossible to cover all different ++ // variants on Windows. ++ if (components.size() >= 2 && ++ (IsLocalUNCServer(components[1]) || ++ components[1] == FILE_PATH_LITERAL("127.0.0.1") || ++ components[1] == FILE_PATH_LITERAL(".") || ++ components[1] == FILE_PATH_LITERAL("?") || ++ ContainsInvalidDNSCharacter(components[1]))) { ++ return true; ++ } ++ ++ // Check *admin* shares only (drive admin like "C$" and named admin). ++ // Note: the share component is typically components[2], but we scan all ++ // components defensively in case the structure changes. ++ for (size_t i = 2; i < components.size(); ++i) { ++ const auto& component = components[i]; ++ ++ // component ends with "$" ++ if (!component.empty() && component.back() == L'$') { ++ // Drive admin share: "C$".."Z$" (case-insensitive on the letter). ++ if (component.size() == 2 && ++ ((component[0] >= L'A' && component[0] <= L'Z') || ++ (component[0] >= L'a' && component[0] <= L'z'))) { ++ return true; ++ } ++ ++ // Named admin shares: "ADMIN$", "IPC$", "PRINT$", and "FAX$" ++ if (base::FilePath::CompareEqualIgnoreCase(component, ++ FILE_PATH_LITERAL("ADMIN$")) || ++ base::FilePath::CompareEqualIgnoreCase(component, ++ FILE_PATH_LITERAL("IPC$")) || ++ base::FilePath::CompareEqualIgnoreCase(component, ++ FILE_PATH_LITERAL("PRINT$")) || ++ base::FilePath::CompareEqualIgnoreCase(component, ++ FILE_PATH_LITERAL("FAX$"))) { ++ return true; ++ } ++ ++ // Otherwise, it is just a hidden share (e.g. "Share$")—do not block. ++ } ++ } ++ ++ return false; ++} ++#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. @@ -370,14 +471,248 @@ index 0000000000000000000000000000000000000000..7cec4f2dd24f5c376293b62aad51994c + + return block_path_rules; +} ++ ++// Checks if `path` should be blocked by the `rules`. ++// The BlockType of the nearest ancestor of a path to check is what ++// ultimately determines if a path is blocked or not. If a blocked path is a ++// descendent of another blocked path, then it may override the ++// child-blocking policy of its ancestor. For example, if /home blocks all ++// children, but /home/downloads does not, then /home/downloads/file.ext ++// will *not* be blocked. ++// static ++bool ChromeFileSystemAccessPermissionContext::ShouldBlockAccessToPath( ++ bool should_normalize_file_path, ++ base::FilePath path, ++ HandleType handle_type, ++ AccessTrigger access_trigger, ++ std::vector ++ extra_rules, ++ ChromeFileSystemAccessPermissionContext::BlockPathRules block_path_rules, ++ base::FilePath profile_path) { ++ CHECK(!path.empty(), base::NotFatalUntil::M161); ++ CHECK(path.IsAbsolute(), base::NotFatalUntil::M161); ++ ++ 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) ++ // On Windows, local UNC paths are rejected, as UNC path can be written in a ++ // way that can bypass the blocklist. ++ if (MaybeIsLocalUNCPath(path)) { ++ return true; ++ } ++#endif ++ ++ base::FilePath nearest_ancestor; ++ BlockType nearest_ancestor_block_type = BlockType::kDontBlockChildren; ++ auto should_block_with_rule = [&](const base::FilePath& block_path, ++ BlockType block_type) -> bool { ++ if (block_type == BlockType::kBlockWrite && ++ access_trigger != AccessTrigger::kSave && ++ access_trigger != AccessTrigger::kProgrammaticWrite) { ++ return false; ++ } ++ ++ if (path == block_path || path.IsParent(block_path)) { ++ LOG(ERROR) << "Blocking access to " << path ++ << " because it is a parent of " << block_path; ++ return true; ++ } ++ ++ if (block_path.IsParent(path) && ++ (nearest_ancestor.empty() || nearest_ancestor.IsParent(block_path))) { ++ nearest_ancestor = block_path; ++ nearest_ancestor_block_type = block_type; ++ } ++ return false; ++ }; ++ ++ for (const auto* block_rules_ptr : ++ {&extra_rules, &block_path_rules.block_path_rules_}) { ++ for (const auto& block : *block_rules_ptr) { ++ if (should_block_with_rule(block.path, block.type)) { ++ return true; ++ } ++ } ++ } ++ ++ for (const auto& rule : block_path_rules.profile_based_block_path_rules_) { ++ if (should_block_with_rule( ++ rule.path ? profile_path.Append(rule.path) : profile_path, ++ rule.type)) { ++ return true; ++ } ++ } ++ ++ std::vector path_components = ++ path.GetComponents(); ++ ++ // Checks if the path components contain the components of the suffix rule. ++ // For example, if the rule is `.git/hooks`, it will block paths like ++ // `/foo/bar/.git/hooks`. The `std::search` identifies the matching subrange ++ // using case-insensitive comparison and constructs a `current_path` from the ++ // root up to the end of the matched subrange (e.g., `/foo/bar/.git/hooks`). ++ // This path is then evaluated against the regular block rules. ++ for (const auto& rule : block_path_rules.suffix_block_path_rules_) { ++ base::FilePath rule_path(rule.path); ++ std::vector rule_components = ++ rule_path.GetComponents(); ++ if (rule_components.empty()) { ++ continue; ++ } ++ ++ auto it = path_components.begin(); ++ while (true) { ++ it = std::search(it, path_components.end(), rule_components.begin(), ++ rule_components.end(), ++ base::FilePath::CompareEqualIgnoreCase); ++ if (it == path_components.end()) { ++ break; ++ } ++ ++ base::FilePath current_path = base::FilePath(path_components[0]); ++ for (auto path_it = path_components.begin() + 1; ++ path_it != it + rule_components.size(); ++path_it) { ++ current_path = current_path.Append(*path_it); ++ } ++ ++ if (should_block_with_rule(current_path, rule.type)) { ++ return true; ++ } ++ ++ ++it; ++ } ++ } ++ ++ // The path we're checking is not in a potentially blocked directory, or the ++ // nearest ancestor does not block access to its children. Grant access. ++ if (nearest_ancestor.empty() || ++ nearest_ancestor_block_type == BlockType::kDontBlockChildren) { ++ VLOG(1) << "Not blocking access to " << path << " because it is inside " ++ << nearest_ancestor << " and it's kDontBlockChildren"; ++ return false; ++ } ++ ++ // The path we're checking is a file, and the nearest ancestor only blocks ++ // access to directories. Grant access. ++ if (handle_type == HandleType::kFile && ++ nearest_ancestor_block_type == BlockType::kBlockNestedDirectories) { ++ VLOG(1) << "Not blocking access to " << path << " because it is inside " ++ << nearest_ancestor << " and it's kBlockNestedDirectories"; ++ return false; ++ } ++ ++ // The nearest ancestor blocks access to its children, so block access. ++ VLOG(1) << "Blocking access to " << path << " because it is inside " ++ << nearest_ancestor << " and it's kBlockAllChildren"; ++ return true; ++} 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 b32c79be552991c5414e5f7317edaee10a2e5193..1a5135f1a06d8f3f4db8477b3e9f25ea0c86ea73 100644 +index b32c79be552991c5414e5f7317edaee10a2e5193..779dd39a4227b61ad2f122eeb5668fe3b7ed03d6 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 -@@ -361,308 +361,8 @@ bool MaybeIsLocalUNCPath(const base::FilePath& path) { +@@ -268,541 +268,8 @@ void ShowFileSystemAccessDangerousFileDialogOnUIThread( } #endif +-#if BUILDFLAG(IS_WIN) +-bool ContainsInvalidDNSCharacter(base::FilePath::StringType hostname) { +- for (base::FilePath::CharType c : hostname) { +- if (!((c >= L'A' && c <= L'Z') || (c >= L'a' && c <= L'z') || +- (c >= L'0' && c <= L'9') || (c == L'.') || (c == L'-') || +- (c == L'_'))) { +- return true; +- } +- } +- return false; +-} +- +-bool IsLocalUNCServer(base::FilePath::StringViewType server) { +- if (server.size() > 1 && server.back() == FILE_PATH_LITERAL('.')) { +- server.remove_suffix(1); +- } +- static constexpr base::FilePath::StringViewType kLocalServers[] = { +- FILE_PATH_LITERAL("localhost"), +- FILE_PATH_LITERAL("wsl.localhost"), +- // Hypervisor host-share redirectors (Parallels, VMware, VirtualBox). +- FILE_PATH_LITERAL("mac"), +- FILE_PATH_LITERAL("psf"), +- FILE_PATH_LITERAL("vmware-host"), +- FILE_PATH_LITERAL(".host"), +- FILE_PATH_LITERAL("vboxsvr"), +- FILE_PATH_LITERAL("vboxsrv"), +- }; +- for (const auto& candidate : kLocalServers) { +- if (base::FilePath::CompareEqualIgnoreCase(server, candidate)) { +- return true; +- } +- } +- return false; +-} +- +-// Returns true if the path is a Universal Naming Convention (UNC) path pointing +-// to a local system path, device namespace, WSL loopback redirector, or +-// hypervisor host-share namespace. +-bool MaybeIsLocalUNCPath(const base::FilePath& path) { +- if (!path.IsNetwork()) { +- return false; +- } +- +- const std::vector components = +- path.GetComponents(); +- +- // Check for server name that could represent a local or host system. We only +- // check for a very short list, as it is impossible to cover all different +- // variants on Windows. +- if (components.size() >= 2 && +- (IsLocalUNCServer(components[1]) || +- components[1] == FILE_PATH_LITERAL("127.0.0.1") || +- components[1] == FILE_PATH_LITERAL(".") || +- components[1] == FILE_PATH_LITERAL("?") || +- ContainsInvalidDNSCharacter(components[1]))) { +- return true; +- } +- +- // Check *admin* shares only (drive admin like "C$" and named admin). +- // Note: the share component is typically components[2], but we scan all +- // components defensively in case the structure changes. +- for (size_t i = 2; i < components.size(); ++i) { +- const auto& component = components[i]; +- +- // component ends with "$" +- if (!component.empty() && component.back() == L'$') { +- // Drive admin share: "C$".."Z$" (case-insensitive on the letter). +- if (component.size() == 2 && +- ((component[0] >= L'A' && component[0] <= L'Z') || +- (component[0] >= L'a' && component[0] <= L'z'))) { +- return true; +- } +- +- // Named admin shares: "ADMIN$", "IPC$", "PRINT$", and "FAX$" +- if (base::FilePath::CompareEqualIgnoreCase(component, +- FILE_PATH_LITERAL("ADMIN$")) || +- base::FilePath::CompareEqualIgnoreCase(component, +- FILE_PATH_LITERAL("IPC$")) || +- base::FilePath::CompareEqualIgnoreCase(component, +- FILE_PATH_LITERAL("PRINT$")) || +- base::FilePath::CompareEqualIgnoreCase(component, +- FILE_PATH_LITERAL("FAX$"))) { +- return true; +- } +- +- // Otherwise, it is just a hidden share (e.g. "Share$")—do not block. +- } +- } +- +- return false; +-} +-#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. @@ -680,33 +1015,154 @@ index b32c79be552991c5414e5f7317edaee10a2e5193..1a5135f1a06d8f3f4db8477b3e9f25ea - return block_path_rules; -} - - // Checks if `path` should be blocked by the `rules`. - // The BlockType of the nearest ancestor of a path to check is what - // ultimately determines if a path is blocked or not. If a blocked path is a -@@ -682,12 +382,15 @@ bool ShouldBlockAccessToPath( - CHECK(!path.empty(), base::NotFatalUntil::M161); - CHECK(path.IsAbsolute(), base::NotFatalUntil::M161); - +-// Checks if `path` should be blocked by the `rules`. +-// The BlockType of the nearest ancestor of a path to check is what +-// ultimately determines if a path is blocked or not. If a blocked path is a +-// descendent of another blocked path, then it may override the +-// child-blocking policy of its ancestor. For example, if /home blocks all +-// children, but /home/downloads does not, then /home/downloads/file.ext +-// will *not* be blocked. +-bool ShouldBlockAccessToPath( +- bool should_normalize_file_path, +- base::FilePath path, +- HandleType handle_type, +- AccessTrigger access_trigger, +- std::vector +- extra_rules, +- ChromeFileSystemAccessPermissionContext::BlockPathRules block_path_rules, +- base::FilePath profile_path) { +- CHECK(!path.empty(), base::NotFatalUntil::M161); +- CHECK(path.IsAbsolute(), base::NotFatalUntil::M161); +- - path = NormalizeFilePathForBlocklist(path, should_normalize_file_path); -+ path = ChromeFileSystemAccessPermissionContext::NormalizeFilePathForBlocklist( -+ path, should_normalize_file_path); - profile_path = +- profile_path = - NormalizeFilePathForBlocklist(profile_path, should_normalize_file_path); -+ ChromeFileSystemAccessPermissionContext::NormalizeFilePathForBlocklist( -+ profile_path, should_normalize_file_path); - for (auto& rule : extra_rules) { - rule.path = +- for (auto& rule : extra_rules) { +- rule.path = - NormalizeFilePathForBlocklist(rule.path, should_normalize_file_path); -+ ChromeFileSystemAccessPermissionContext::NormalizeFilePathForBlocklist( -+ rule.path, should_normalize_file_path); - } - - #if BUILDFLAG(IS_WIN) +- } +- +-#if BUILDFLAG(IS_WIN) +- // On Windows, local UNC paths are rejected, as UNC path can be written in a +- // way that can bypass the blocklist. +- if (MaybeIsLocalUNCPath(path)) { +- return true; +- } +-#endif +- +- base::FilePath nearest_ancestor; +- BlockType nearest_ancestor_block_type = BlockType::kDontBlockChildren; +- auto should_block_with_rule = [&](const base::FilePath& block_path, +- BlockType block_type) -> bool { +- if (block_type == BlockType::kBlockWrite && +- access_trigger != AccessTrigger::kSave && +- access_trigger != AccessTrigger::kProgrammaticWrite) { +- return false; +- } +- +- if (path == block_path || path.IsParent(block_path)) { +- LOG(ERROR) << "Blocking access to " << path +- << " because it is a parent of " << block_path; +- return true; +- } +- +- if (block_path.IsParent(path) && +- (nearest_ancestor.empty() || nearest_ancestor.IsParent(block_path))) { +- nearest_ancestor = block_path; +- nearest_ancestor_block_type = block_type; +- } +- return false; +- }; +- +- for (const auto* block_rules_ptr : +- {&extra_rules, &block_path_rules.block_path_rules_}) { +- for (const auto& block : *block_rules_ptr) { +- if (should_block_with_rule(block.path, block.type)) { +- return true; +- } +- } +- } +- +- for (const auto& rule : block_path_rules.profile_based_block_path_rules_) { +- if (should_block_with_rule( +- rule.path ? profile_path.Append(rule.path) : profile_path, +- rule.type)) { +- return true; +- } +- } +- +- std::vector path_components = +- path.GetComponents(); +- +- // Checks if the path components contain the components of the suffix rule. +- // For example, if the rule is `.git/hooks`, it will block paths like +- // `/foo/bar/.git/hooks`. The `std::search` identifies the matching subrange +- // using case-insensitive comparison and constructs a `current_path` from the +- // root up to the end of the matched subrange (e.g., `/foo/bar/.git/hooks`). +- // This path is then evaluated against the regular block rules. +- for (const auto& rule : block_path_rules.suffix_block_path_rules_) { +- base::FilePath rule_path(rule.path); +- std::vector rule_components = +- rule_path.GetComponents(); +- if (rule_components.empty()) { +- continue; +- } +- +- auto it = path_components.begin(); +- while (true) { +- it = std::search(it, path_components.end(), rule_components.begin(), +- rule_components.end(), +- base::FilePath::CompareEqualIgnoreCase); +- if (it == path_components.end()) { +- break; +- } +- +- base::FilePath current_path = base::FilePath(path_components[0]); +- for (auto path_it = path_components.begin() + 1; +- path_it != it + rule_components.size(); ++path_it) { +- current_path = current_path.Append(*path_it); +- } +- +- if (should_block_with_rule(current_path, rule.type)) { +- return true; +- } +- +- ++it; +- } +- } +- +- // The path we're checking is not in a potentially blocked directory, or the +- // nearest ancestor does not block access to its children. Grant access. +- if (nearest_ancestor.empty() || +- nearest_ancestor_block_type == BlockType::kDontBlockChildren) { +- VLOG(1) << "Not blocking access to " << path << " because it is inside " +- << nearest_ancestor << " and it's kDontBlockChildren"; +- return false; +- } +- +- // The path we're checking is a file, and the nearest ancestor only blocks +- // access to directories. Grant access. +- if (handle_type == HandleType::kFile && +- nearest_ancestor_block_type == BlockType::kBlockNestedDirectories) { +- VLOG(1) << "Not blocking access to " << path << " because it is inside " +- << nearest_ancestor << " and it's kBlockNestedDirectories"; +- return false; +- } +- +- // The nearest ancestor blocks access to its children, so block access. +- VLOG(1) << "Blocking access to " << path << " because it is inside " +- << nearest_ancestor << " and it's kBlockAllChildren"; +- return true; +-} +- + // Returns true if `child_path` is the same as or a descendant of + // `parent_path`, ignoring case differences. Unlike + // `base::FilePath::IsParent()`, this handles case-variant paths returned by diff --git a/chrome/browser/file_system_access/chrome_file_system_access_permission_context.h b/chrome/browser/file_system_access/chrome_file_system_access_permission_context.h -index 05ee182cf840bc925697a2bfa3d1c281579296c4..33595aaa73cf1301f1361bd07deb39ac30cc9201 100644 +index 05ee182cf840bc925697a2bfa3d1c281579296c4..e201e4d82b7298d6f90d78490aa342d915251a56 100644 --- a/chrome/browser/file_system_access/chrome_file_system_access_permission_context.h +++ b/chrome/browser/file_system_access/chrome_file_system_access_permission_context.h -@@ -242,6 +242,18 @@ class ChromeFileSystemAccessPermissionContext +@@ -242,6 +242,31 @@ class ChromeFileSystemAccessPermissionContext } }; @@ -721,6 +1177,19 @@ index 05ee182cf840bc925697a2bfa3d1c281579296c4..33595aaa73cf1301f1361bd07deb39ac + // chrome_file_system_access_blocklist.cc. + static std::unique_ptr GenerateBlockPaths( + bool should_normalize_file_path); ++ ++ // Returns true if access to `path` should be blocked by `extra_rules` or ++ // `block_path_rules`. Rules of type `BlockType::kBlockWrite` only apply when ++ // `access_trigger` is `AccessTrigger::kSave` or ++ // `AccessTrigger::kProgrammaticWrite`. May block, so must be called on a ++ // sequence that allows blocking. See chrome_file_system_access_blocklist.cc. ++ static bool ShouldBlockAccessToPath(bool should_normalize_file_path, ++ base::FilePath path, ++ HandleType handle_type, ++ AccessTrigger access_trigger, ++ std::vector extra_rules, ++ BlockPathRules block_path_rules, ++ base::FilePath profile_path); + explicit ChromeFileSystemAccessPermissionContext( content::BrowserContext* context, diff --git a/shell/browser/file_system_access/file_system_access_permission_context.cc b/shell/browser/file_system_access/file_system_access_permission_context.cc index 4c83f6eb76..c7d515754d 100644 --- a/shell/browser/file_system_access/file_system_access_permission_context.cc +++ b/shell/browser/file_system_access/file_system_access_permission_context.cc @@ -84,7 +84,6 @@ struct Converter< namespace { -using BlockType = ChromeFileSystemAccessPermissionContext::BlockType; using HandleType = content::FileSystemAccessPermissionContext::HandleType; using GrantType = electron::FileSystemAccessPermissionContext::GrantType; using SensitiveEntryResult = @@ -112,131 +111,6 @@ constexpr std::string_view kTimestampKey = "timestamp"; constexpr base::TimeDelta kPermissionRevocationTimeout = base::Seconds(5); -#if BUILDFLAG(IS_WIN) -[[nodiscard]] constexpr bool ContainsInvalidDNSCharacter( - base::FilePath::StringType hostname) { - return !std::ranges::all_of(hostname, [](base::FilePath::CharType c) { - return (c >= L'A' && c <= L'Z') || (c >= L'a' && c <= L'z') || - (c >= L'0' && c <= L'9') || (c == L'.') || (c == L'-'); - }); -} - -// Returns true if the path is a Universal Naming Convention (UNC) path pointing -// to a local system path, device namespace, or WSL loopback redirector. -bool MaybeIsLocalUNCPath(const base::FilePath& path) { - if (!path.IsNetwork()) { - return false; - } - - const std::vector components = - path.GetComponents(); - - // Check for server name that could represent a local system. We only - // check for a very short list, as it is impossible to cover all different - // variants on Windows. - if (components.size() >= 2 && - (base::FilePath::CompareEqualIgnoreCase(components[1], - FILE_PATH_LITERAL("localhost")) || - base::FilePath::CompareEqualIgnoreCase( - components[1], FILE_PATH_LITERAL("wsl.localhost")) || - base::FilePath::CompareEqualIgnoreCase( - components[1], FILE_PATH_LITERAL("wsl.localhost.")) || - components[1] == FILE_PATH_LITERAL("127.0.0.1") || - components[1] == FILE_PATH_LITERAL(".") || - components[1] == FILE_PATH_LITERAL("?") || - ContainsInvalidDNSCharacter(components[1]))) { - return true; - } - - // In case we missed the server name check above, we also check for shares - // ending with '$' as they represent pre-defined shares, including the local - // drives. - for (size_t i = 2; i < components.size(); ++i) { - if (components[i].back() == L'$') { - return true; - } - } - - return false; -} -#endif // BUILDFLAG(IS_WIN) - -bool ShouldBlockAccessToPath( - base::FilePath path, - HandleType handle_type, - std::vector - extra_rules, - ChromeFileSystemAccessPermissionContext::BlockPathRules block_path_rules) { - DCHECK(!path.empty()); - DCHECK(path.IsAbsolute()); - - path = ChromeFileSystemAccessPermissionContext::NormalizeFilePathForBlocklist( - path, /*should_normalize_file_path=*/true); - for (auto& rule : extra_rules) { - rule.path = - ChromeFileSystemAccessPermissionContext::NormalizeFilePathForBlocklist( - rule.path, /*should_normalize_file_path=*/true); - } - -#if BUILDFLAG(IS_WIN) - // On Windows, local UNC paths are rejected, as UNC path can be written in a - // way that can bypass the blocklist. - if (MaybeIsLocalUNCPath(path)) { - return true; - } -#endif - - base::FilePath nearest_ancestor; - BlockType nearest_ancestor_block_type = BlockType::kDontBlockChildren; - auto should_block_with_rule = [&](const base::FilePath& block_path, - BlockType block_type) -> bool { - if (path == block_path || path.IsParent(block_path)) { - VLOG(1) << "Blocking access to " << path << " because it is a parent of " - << block_path; - return true; - } - - if (block_path.IsParent(path) && - (nearest_ancestor.empty() || nearest_ancestor.IsParent(block_path))) { - nearest_ancestor = block_path; - nearest_ancestor_block_type = block_type; - } - return false; - }; - - for (const auto* block_rules_ptr : - {&extra_rules, &block_path_rules.block_path_rules_}) { - for (const auto& block : *block_rules_ptr) { - if (should_block_with_rule(block.path, block.type)) { - return true; - } - } - } - - // The path we're checking is not in a potentially blocked directory, or the - // nearest ancestor does not block access to its children. Grant access. - if (nearest_ancestor.empty() || - nearest_ancestor_block_type == BlockType::kDontBlockChildren) { - VLOG(1) << "Not blocking access to " << path << " because it is inside " - << nearest_ancestor << " and it's kDontBlockChildren"; - return false; - } - - // The path we're checking is a file, and the nearest ancestor only blocks - // access to directories. Grant access. - if (handle_type == HandleType::kFile && - nearest_ancestor_block_type == BlockType::kBlockNestedDirectories) { - VLOG(1) << "Not blocking access to " << path << " because it is inside " - << nearest_ancestor << " and it's kBlockNestedDirectories"; - return false; - } - - // The nearest ancestor blocks access to its children, so block access. - VLOG(1) << "Blocking access to " << path << " because it is inside " - << nearest_ancestor << " and it's kBlockAllChildren"; - return true; -} - std::string GenerateLastPickedDirectoryKey(const std::string& id) { return id.empty() ? kDefaultLastPickedDirectoryKey : base::StrCat({kCustomLastPickedDirectoryKey, "-", id}); @@ -722,27 +596,36 @@ void FileSystemAccessPermissionContext::ConfirmSensitiveEntryAccess( &FileSystemAccessPermissionContext::DidCheckPathAgainstBlocklist, GetWeakPtr(), request_id, origin, path_info, handle_type, access_trigger, frame_id); - CheckPathAgainstBlocklist(path_info, handle_type, + CheckPathAgainstBlocklist(path_info, handle_type, access_trigger, std::move(after_blocklist_check_callback)); } void FileSystemAccessPermissionContext::CheckShouldBlockAccessToPathAndReply( base::FilePath path, HandleType handle_type, + AccessTrigger access_trigger, std::vector extra_rules, base::OnceCallback callback, ChromeFileSystemAccessPermissionContext::BlockPathRules block_path_rules) { + // Uses Chrome's own matcher so that suffix rules, kBlockWrite rules (which + // only apply to AccessTrigger::kSave and AccessTrigger::kProgrammaticWrite) + // and the Windows local UNC path checks stay in sync with upstream. The + // profile path is only consulted for ChromeOS profile-based rules. base::ThreadPool::PostTaskAndReplyWithResult( FROM_HERE, {base::MayBlock(), base::TaskPriority::USER_VISIBLE}, - base::BindOnce(&ShouldBlockAccessToPath, path, handle_type, extra_rules, - block_path_rules), + base::BindOnce( + &ChromeFileSystemAccessPermissionContext::ShouldBlockAccessToPath, + /*should_normalize_file_path=*/true, std::move(path), handle_type, + access_trigger, std::move(extra_rules), std::move(block_path_rules), + browser_context()->GetPath()), std::move(callback)); } void FileSystemAccessPermissionContext::CheckPathAgainstBlocklist( const content::PathInfo& path_info, HandleType handle_type, + AccessTrigger access_trigger, base::OnceCallback callback) { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); // TODO(crbug.com/40101272): Figure out what external paths should be @@ -765,9 +648,9 @@ void FileSystemAccessPermissionContext::CheckPathAgainstBlocklist( if (is_block_path_rules_init_complete_) { // The rules initialization is completed, we can just post the task to a // anonymous blocking traits. - CheckShouldBlockAccessToPathAndReply(path_info.path, handle_type, - extra_rules, std::move(callback), - *block_path_rules_.get()); + CheckShouldBlockAccessToPathAndReply( + path_info.path, handle_type, access_trigger, extra_rules, + std::move(callback), *block_path_rules_.get()); return; } // The check must be performed after the rules initialization is done. @@ -775,7 +658,7 @@ void FileSystemAccessPermissionContext::CheckPathAgainstBlocklist( base::BindOnce(&FileSystemAccessPermissionContext:: CheckShouldBlockAccessToPathAndReply, weak_factory_.GetWeakPtr(), path_info.path, handle_type, - extra_rules, std::move(callback)))); + access_trigger, extra_rules, std::move(callback)))); } void FileSystemAccessPermissionContext::PerformAfterWriteChecks( diff --git a/shell/browser/file_system_access/file_system_access_permission_context.h b/shell/browser/file_system_access/file_system_access_permission_context.h index 1c2aa4cefa..6e01bfdb53 100644 --- a/shell/browser/file_system_access/file_system_access_permission_context.h +++ b/shell/browser/file_system_access/file_system_access_permission_context.h @@ -138,6 +138,7 @@ class FileSystemAccessPermissionContext void CheckShouldBlockAccessToPathAndReply( base::FilePath path, HandleType handle_type, + AccessTrigger access_trigger, std::vector extra_rules, base::OnceCallback callback, @@ -145,6 +146,7 @@ class FileSystemAccessPermissionContext void CheckPathAgainstBlocklist(const content::PathInfo& path, HandleType handle_type, + AccessTrigger access_trigger, base::OnceCallback callback); void DidCheckPathAgainstBlocklist(int request_id, const url::Origin& origin, diff --git a/spec/chromium.spec.ts b/spec/chromium.spec.ts index 2c70336f58..890cf8d633 100644 --- a/spec/chromium.spec.ts +++ b/spec/chromium.spec.ts @@ -2028,6 +2028,161 @@ describe('chromium features', () => { expect(restricted).toHaveLength(2); }); + it('blocks writes to .git/hooks, .git/config and .git/modules but still allows reading them', async () => { + const ses = session.fromPartition(`fsa-scope-${Math.random()}`); + ses.setPermissionRequestHandler((_wc, _permission, callback) => callback(true)); + const restricted: string[] = []; + ses.on('file-system-access-restricted', (_e, details, callback) => { + restricted.push(details.path); + callback('deny'); + }); + + // On Windows the temp dir lives under %LOCALAPPDATA%, which the blocklist + // blocks entirely, so use a directory the blocklist leaves alone there. + const base = process.platform === 'win32' ? path.join(fixturesPath, 'file-system') : os.tmpdir(); + const dir = fs.realpathSync(fs.mkdtempSync(path.join(base, 'fsa-git-'))); + defer(() => fs.rmSync(dir, { recursive: true, force: true })); + const hooks = path.join(dir, '.git', 'hooks'); + fs.mkdirSync(hooks, { recursive: true }); + fs.writeFileSync(path.join(hooks, 'pre-commit.sample'), 'sample'); + fs.writeFileSync(path.join(dir, '.git', 'config'), '[core]\n'); + + const w = new BrowserWindow({ show: true, webPreferences: { session: ses } }); + await w.loadURL(urlA); + const frame = w.webContents.mainFrame; + await pasteHandle(w, frame, dir); + await waitForHandle(frame); + expect(await frame.executeJavaScript('window.handle && window.handle.kind')).toBe('directory'); + expect(await frame.executeJavaScript("window.handle.requestPermission({ mode: 'readwrite' })", true)).toBe( + 'granted' + ); + + // Matches Chrome: these are kBlockWrite suffix rules, so reads are allowed + // and only operations that write are rejected. + const result = await frame.executeJavaScript(`(async () => { + const attempt = (p) => p.then(() => 'ok', (e) => e.name); + const git = await window.handle.getDirectoryHandle('.git'); + const hooks = await git.getDirectoryHandle('hooks'); + const sample = await hooks.getFileHandle('pre-commit.sample'); + const config = await git.getFileHandle('config'); + return { + readHook: await (await sample.getFile()).text(), + readConfig: await (await config.getFile()).text(), + createHook: await attempt(hooks.getFileHandle('post-checkout', { create: true })), + writeHook: await attempt(sample.createWritable()), + writeConfig: await attempt(config.createWritable()), + createModules: await attempt(git.getDirectoryHandle('modules', { create: true })), + createAllowed: await attempt(window.handle.getFileHandle('allowed.txt', { create: true })) + }; + })()`); + expect(result).toEqual({ + readHook: 'sample', + readConfig: '[core]\n', + createHook: 'SecurityError', + writeHook: 'SecurityError', + writeConfig: 'SecurityError', + createModules: 'SecurityError', + createAllowed: 'ok' + }); + expect(fs.existsSync(path.join(hooks, 'post-checkout'))).toBe(false); + expect(fs.existsSync(path.join(dir, '.git', 'modules'))).toBe(false); + expect(fs.readFileSync(path.join(dir, '.git', 'config'), 'utf8')).toBe('[core]\n'); + expect(fs.existsSync(path.join(dir, 'allowed.txt'))).toBe(true); + // Programmatic writes are rejected outright rather than asking the app. + expect(restricted).toEqual([]); + }); + + // A pasted directory only reaches the blocklist if it exists, so this + // shares a fixture directory over loopback SMB and reaches it under + // different UNC server and share names. Creating a share needs an + // elevated administrator. + ifit(process.platform === 'win32')("applies Chrome's local UNC path rules", { timeout: 120000 }, async (ctx) => { + const dir = fs.realpathSync(fs.mkdtempSync(path.join(fixturesPath, 'file-system', 'fsa-unc-'))); + defer(() => fs.rmSync(dir, { recursive: true, force: true })); + // At most 12 characters, including the hidden share's '$', or net share + // asks whether MS-DOS clients matter. + const share = `efsa${Math.floor(Math.random() * 0x1000000) + .toString(16) + .padStart(6, '0')}`; + const hiddenShare = `${share}$`; + try { + for (const name of [share, hiddenShare]) { + ChildProcess.execFileSync('net', ['share', `${name}=${dir}`], { stdio: 'pipe', input: 'y\r\n' }); + defer(() => { + try { + ChildProcess.execFileSync('net', ['share', name, '/delete', '/y'], { stdio: 'pipe' }); + } catch {} + }); + } + } catch (error) { + // On CI this must not turn into a silent skip. + if (process.env.CI) throw error; + console.warn(`Skipping UNC blocklist spec, could not create an SMB share: ${error}`); + return ctx.skip(); + } + + const host = os.hostname(); + const unc = (server: string, ...rest: string[]) => `\\\\${server}\\${rest.join('\\')}`; + const isReachable = (p: string) => + Promise.race([ + fs.promises.stat(p).then( + (s) => s.isDirectory(), + () => false + ), + setTimeout(20000, false, { ref: false }) + ]); + + const ses = session.fromPartition(`fsa-scope-${Math.random()}`); + const restricted: string[] = []; + ses.on('file-system-access-restricted', (_e, details, callback) => { + restricted.push(details.path); + callback('deny'); + }); + const w = new BrowserWindow({ show: true, webPreferences: { session: ses } }); + await w.loadURL(urlA); + const frame = w.webContents.mainFrame; + const classify = async (p: string) => { + // Also opens the SMB session up front, which can be slow. + expect(await isReachable(p), `${p} should be reachable`).toBe(true); + const before = restricted.length; + await frame.executeJavaScript('window.handle = null; true'); + await pasteHandle(w, frame, p); + await waitForHandle(frame); + const kind = await frame.executeJavaScript('window.handle && window.handle.kind'); + if (restricted.length === before) { + expect(kind, p).toBe('directory'); + return 'allowed'; + } + // File URLs lowercase the server name. + expect(restricted.slice(before).map((r) => r.toLowerCase())).toEqual([p.toLowerCase()]); + expect(kind, p).toBe(null); + return 'blocked'; + }; + + // A server name that isn't known to be local is treated as remote. + expect(await classify(unc(host, share))).toBe('allowed'); + // Intentional change: Electron used to block every share ending in '$'; + // like Chrome, only drive (X$) and ADMIN$/IPC$/PRINT$/FAX$ shares are. + expect(await classify(unc(host, hiddenShare))).toBe('allowed'); + // Drive admin shares are blocked whatever the server is called. + const [drive, ...dirComponents] = dir.split(path.sep); + expect(await classify(unc(host, `${drive.replace(/:$/, '')}$`, ...dirComponents))).toBe('blocked'); + // Local server names are blocked whatever the share is. A bare + // "localhost" can't be pasted: file URLs drop that host, so it would + // arrive as a path with no server. + expect(await classify(unc('127.0.0.1', share))).toBe('blocked'); + + // Intentional change: a trailing dot no longer hides a local server name. + // Electron used to allow this. It needs "localhost." to resolve, so it is + // skipped where it doesn't. + const trailingDot = unc('localhost.', share); + if (await isReachable(trailingDot)) { + expect(await classify(trailingDot)).toBe('blocked'); + } else { + console.warn(`Skipping ${trailingDot}, it does not resolve here`); + } + }); + it('revokes active grants once no top-level document of the origin remains', { timeout: 60000 }, async () => { const ses = session.fromPartition(`fsa-scope-${Math.random()}`); ses.setPermissionRequestHandler((_wc, _permission, callback) => callback(true));