Chrome · FileSystem › FSA
CVE-2026-113175
Input Validation in FileSystem (FSA)
Overview
High
Severity
—
CVSS
No
Exploited ITW
Fixed
Fix Status
Changed Functions
| Function | Change | Notes |
|---|---|---|
ifcontent/browser/file_system_access/file_system_access_manager_impl.cc |
modified | |
forcontent/browser/file_system_access/file_system_access_manager_impl_unittest.cc |
modified |
Files Changed
content/browser/file_system_access/file_system_access_manager_impl_unittest.cccontent/test/BUILD.gncontent/browser/file_system_access/file_system_access_manager_impl.cc
Patch
From 2d04a51fffd055b2b065b13853c225259aa6882d Mon Sep 17 00:00:00 2001
From: Eriko Kurimoto <elkurin@google.com>
Date: Wed, 07 Oct 2026 22:05:45 -0700
Subject: [PATCH] [FSA] Validate paths in DeserializeHandle
Previously, FileSystemAccessManagerImpl::DeserializeHandle constructed
file system URLs from serialized handle protobufs without validating
root or relative path invariants. Non-relative child paths, parent
directory references, current-directory segments, or invalid root paths
could resolve outside the root permission path, alias paths in lock
management, or fail URL cracking invariants. Additionally, an unset
protobuf data case triggered a NOTREACHED crash.
Validate root and relative paths in DeserializeHandle before creating
transfer tokens. Reject empty or parent-referencing root paths, require
local root paths to be absolute or Android content URIs, and reject
relative paths containing parent references, current-directory segments,
root separators, or drive-letter prefixes. Verify that non-empty
relative paths resolve underneath the root URL, and drop the transfer
token when the protobuf data case is unset.
For sandboxed handles, reject non-plain relative paths (including parent
references and current-directory segments) and invalid URLs.
ObfuscatedFileUtil strips '.' via VirtualPath::GetComponents(), whereas
FileSystemAccessLockManager preserves '.' via FilePath::GetComponents(),
so allowing '.' segments would alias paths without sharing locks.
Bug: 568822820
Test: content_unittests --gtest_filter=*FileSystemAccessManagerImpl*
Change-Id: Iab2944102acadf172349e0e0caf2422760ebb269
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8529346
Reviewed-by: Ming-Ying Chung <mych@chromium.org>
Commit-Queue: Eriko Kurimoto <elkurin@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1714162}
---
diff --git a/content/browser/file_system_access/file_system_access_manager_impl.cc b/content/browser/file_system_access/file_system_access_manager_impl.cc
index 5935297d..8760e71 100644
--- a/content/browser/file_system_access/file_system_access_manager_impl.cc
+++ b/content/browser/file_system_access/file_system_access_manager_impl.cc
@@ -1175,6 +1175,45 @@
return base::FilePath(s);
}
+bool IsValidRootPath(FileSystemAccessHandleData::DataCase data_case,
+ const base::FilePath& root_path) {
+ if (root_path.empty() || root_path.ReferencesParent()) {
+ return false;
+ }
+ if (data_case == FileSystemAccessHandleData::kLocal) {
+#if BUILDFLAG(IS_ANDROID)
+ if (root_path.IsContentUri()) {
+ return true;
+ }
+#endif
+ return root_path.IsAbsolute();
+ }
+ return true;
+}
+
+bool IsPlainRelativePath(const base::FilePath& path) {
+ if (path.empty()) {
+ return true;
+ }
+ if (path.IsAbsolute() || path.ReferencesParent()) {
+ return false;
+ }
+ // `FilePath::GetComponents()` drops a leading `.`, so check the prefix first.
+ if (path.value().starts_with(base::FilePath::kCurrentDirectory) &&
+ (path.value().size() == 1 ||
+ base::FilePath::IsSeparator(path.value()[1]))) {
+ return false;
+ }
+ for (const auto& component : path.GetComponents()) {
+ if (component.empty() || component == base::FilePath::kCurrentDirectory ||
+ base::FilePath::IsSeparator(component[0]) ||
+ base::FilePath(component).BaseName().value() != component) {
+ return false;
+ }
+ }
+ return true;
+}
+
std::string SerializeURLImpl(const storage::FileSystemURL& url,
FileSystemAccessPermissionContext::HandleType type,
base::FilePath root_permission_path,
@@ -1303,8 +1342,21 @@
case FileSystemAccessHandleData::kSandboxed: {
base::FilePath virtual_path =
DeserializePath(data.sandboxed().virtual_path());
+ // Reject non-plain relative paths, including `..` and `.` segments.
+ // `ObfuscatedFileUtil` strips `.` via `VirtualPath::GetComponents()`,
+ // while `FileSystemAccessLockManager` preserves `.` via
+ // `FilePath::GetComponents()`, which would alias paths without sharing
+ // locks.
+ if (!IsPlainRelativePath(virtual_path)) {
+ // Drop `token`, and directly return.
+ return;
+ }
storage::FileSystemURL url = context()->CreateCrackedFileSystemURL(
storage_key, storage::kFileSystemTypeTemporary, virtual_path);
+ if (!url.is_valid()) {
+ // Drop `token`, and directly return.
+ return;
+ }
// Apply bucket information.
auto bucket_callback = base::BindOnce(
[](storage::FileSystemURL url,
@@ -1347,18 +1399,44 @@
base::FilePath root_path = DeserializePath(file_data.root_path());
base::FilePath relative_path = DeserializePath(file_data.relative_path());
+ if (!IsValidRootPath(data.data_case(), root_path) ||
+ !IsPlainRelativePath(relative_path)) {
+ // Drop `token`, and directly return.
+ return;
+ }
+
+ std::string display_name = !file_data.display_name().empty()
+ ? file_data.display_name()
+ : root_path.BaseName().AsUTF8Unsafe();
+ // `BaseName()` can be empty on Windows when `root_path` is a bare drive
+ // letter (e.g., `kExternal` with `root_path = L"C:"`), which would fail
+ // the `CHECK` in `PathInfo`.
+ if (display_name.empty()) {
+ // Drop `token`, and directly return.
+ return;
+ }
+
PathInfo path_info(data.data_case() == FileSystemAccessHandleData::kLocal
? PathType::kLocal
: PathType::kExternal,
- root_path,
- !file_data.display_name().empty()
- ? file_data.display_name()
- : root_path.BaseName().AsUTF8Unsafe());
+ root_path, std::move(display_name));
storage::FileSystemURL root = CreateFileSystemURLFromPath(path_info);
+ if (!root.is_valid()) {
+ // Drop `token`, and directly return.
+ return;
+ }
storage::FileSystemURL child = context()->CreateCrackedFileSystemURL(
root.storage_key(), root.mount_type(),
root.virtual_path().Append(relative_path));
+ // `IsPlainRelativePath()` already guarantees containment; `IsParent()` is
+ // kept as an additional consistency check.
+ if (!child.is_valid() ||
+ (!relative_path.empty() &&
+ !root.virtual_path().IsParent(child.virtual_path()))) {
+ // Drop `token`, and directly return.
+ return;
+ }
const bool is_directory =
data.handle_type() == FileSystemAccessHandleData::kDirectory;
@@ -1379,7 +1457,8 @@
break;
}
case FileSystemAccessHandleData::DATA_NOT_SET:
- NOTREACHED();
+ // Drop `token`, and directly return.
+ return;
}
}
diff --git a/content/browser/file_system_access/file_system_access_manager_impl_unittest.cc b/content/browser/file_system_access/file_system_access_manager_impl_unittest.cc
index 4891438..e240e46a 100644
--- a/content/browser/file_system_access/file_system_access_manager_impl_unittest.cc
+++ b/content/browser/file_system_access/file_system_access_manager_impl_unittest.cc
@@ -31,6 +31,7 @@
#include "components/services/storage/public/cpp/buckets/constants.h"
#include "content/browser/blob_storage/chrome_blob_storage_context.h"
#include "content/browser/file_system_access/features.h"
+#include "content/browser/file_system_access/file_system_access.pb.h"
#include "content/browser/file_system_access/file_system_access_data_transfer_token_impl.h"
#include "content/browser/file_system_access/file_system_access_directory_handle_impl.h"
#include "content/browser/file_system_access/file_system_access_file_handle_impl.h"
@@ -413,6 +414,29 @@
return resolve_future.Get();
}
+ std::string SerializePathForTesting(const base::FilePath& path) {
+ auto path_bytes = base::as_byte_span(path.value());
+ return std::string(path_bytes.begin(), path_bytes.end());
+ }
+
+ FileSystemAccessTransferTokenImpl* DeserializeAndResolveToken(
+ const FileSystemAccessHandleData& data) {
+ std::string serialized_string;
+ EXPECT_TRUE(data.SerializeToString(&serialized_string));
+ std::vector<uint8_t> serialized(serialized_string.begin(),
+ serialized_string.end());
+ EXPECT_FALSE(serialized.empty());
+
+ mojo::PendingRemote<blink::mojom::FileSystemAccessTransferToken>
+ token_remote;
+ manager_->DeserializeHandle(kTestStorageKey, serialized,
+ token_remote.InitWithNewPipeAndPassReceiver());
+ base::test::TestFuture<FileSystemAccessTransferTokenImpl*> resolve_future;
+ manager_->ResolveTransferToken(std::move(token_remote),
+ resolve_future.GetCallback());
+ return resolve_future.Get();
+ }
+
void GetEntryFromDataTransferTokenFileTest(
const PathInfo& file_path_info,
const std::string& expected_file_contents) {
@@ -1822,6 +1846,183 @@
EXPECT_EQ(ask_grant2_, token->GetWriteGrant());
}
+TEST_F(FileSystemAccessManagerImplTest,
+ DeserializeHandle_Native_InvalidRelativePath) {
+ const base::FilePath root_path = dir_.GetPath().AppendASCII("foo");
+ const base::FilePath invalid_relative_paths[] = {
+ base::FilePath(FILE_PATH_LITERAL("..")),
+ base::FilePath(FILE_PATH_LITERAL("../bar")),
+ base::FilePath(FILE_PATH_LITERAL("../other_dir")),
+ base::FilePath(FILE_PATH_LITERAL("sub/../../bar")),
+ base::FilePath(FILE_PATH_LITERAL(".")),
+ base::FilePath(FILE_PATH_LITERAL("./bar")),
+ dir_.GetPath().AppendASCII("bar"),
+#if BUILDFLAG(IS_WIN)
+ base::FilePath(FILE_PATH_LITERAL("C:bar")),
+ base::FilePath(FILE_PATH_LITERAL("\\bar")),
+#endif
+ };
+
+ EXPECT_CALL(permission_context_, GetReadPermissionGrant).Times(0);
+ EXPECT_CALL(permission_context_, GetWritePermissionGrant).Times(0);
+
+ for (auto handle_type : {FileSystemAccessHandleData::kFile,
+ FileSystemAccessHandleData::kDirectory}) {
+ for (const auto& relative_path : invalid_relative_paths) {
+ FileSystemAccessHandleData data;
+ data.set_handle_type(handle_type);
+ data.mutable_local()->set_root_path(SerializePathForTesting(root_path));
+ data.mutable_local()->set_relative_path(
+ SerializePathForTesting(relative_path));
+ EXPECT_FALSE(DeserializeAndResolveToken(data)) << relative_path;
+ }
+ }
+}
+
+TEST_F(FileSystemAccessManagerImplTest,
+ DeserializeHandle_Native_InvalidRootPath) {
+ const base::FilePath invalid_root_paths[] = {
+ base::FilePath(),
+ dir_.GetPath().AppendASCII("foo").Append(FILE_PATH_LITERAL("../bar")),
+ base::FilePath(FILE_PATH_LITERAL("relative/root")),
+ };
+
+ EXPECT_CALL(permission_context_, GetReadPermissionGrant).Times(0);
+ EXPECT_CALL(permission_context_, GetWritePermissionGrant).Times(0);
+
+ for (const auto& root_path : invalid_root_paths) {
+ FileSystemAccessHandleData data;
+ data.set_handle_type(FileSystemAccessHandleData::kDirectory);
+ data.mutable_local()->set_root_path(SerializePathForTesting(root_path));
+ data.mutable_local()->set_relative_path(
+ SerializePathForTesting(base::FilePath()));
+ EXPECT_FALSE(DeserializeAndResolveToken(data)) << root_path;
+ }
+}
+
+TEST_F(FileSystemAccessManagerImplTest,
+ DeserializeHandle_External_InvalidPaths) {
+ EXPECT_CALL(permission_context_, GetReadPermissionGrant).Times(0);
+ EXPECT_CALL(permission_context_, GetWritePermissionGrant).Times(0);
+
+ {
+ FileSystemAccessHandleData data;
+ data.set_handle_type(FileSystemAccessHandleData::kFile);
+ data.mutable_external()->set_root_path(SerializePathForTesting(
+ base::FilePath::FromUTF8Unsafe(kTestMountPoint).AppendASCII("foo")));
+ data.mutable_external()->set_relative_path(
+ SerializePathForTesting(base::FilePath(FILE_PATH_LITERAL("../bar"))));
+ EXPECT_FALSE(DeserializeAndResolveToken(data));
+ }
+
+ {
+ FileSystemAccessHandleData data;
+ data.set_handle_type(FileSystemAccessHandleData::kFile);
+ data.mutable_external()->set_root_path(
+ SerializePathForTesting(base::FilePath::FromUTF8Unsafe(kTestMountPoint)
+ .Append(FILE_PATH_LITERAL("../foo"))));
+ data.mutable_external()->set_relative_path(
+ SerializePathForTesting(base::FilePath()));
+ EXPECT_FALSE(DeserializeAndResolveToken(data));
+ }
+
+ // Unregistered external mounts produce an invalid `root` URL. Without the
+ // `root.is_valid()` check, `CreateCrackedFileSystemURL` on
+ // `kFileSystemTypeUnknown` returns a URL with `is_valid() == true`.
+ {
+ FileSystemAccessHandleData data;
+ data.set_handle_type(FileSystemAccessHandleData::kFile);
+ data.mutable_external()->set_root_path(SerializePathForTesting(
+ base::FilePath::FromUTF8Unsafe("unregistered_mount")
+ .AppendASCII("foo")));
+ data.mutable_external()->set_relative_path(
+ SerializePathForTesting(base::FilePath()));
+ EXPECT_FALSE(DeserializeAndResolveToken(data));
+ }
+
+#if BUILDFLAG(IS_WIN)
+ // A bare drive letter passes `IsValidRootPath` for `kExternal`, but
+ // `FilePath::BaseName()` strips the drive letter and returns an empty string.
+ {
+ FileSystemAccessHandleData data;
+ data.set_handle_type(FileSystemAccessHandleData::kDirectory);
+ data.mutable_external()->set_root_path(
+ SerializePathForTesting(base::FilePath(FILE_PATH_LITERAL("C:"))));
+ data.mutable_external()->set_relative_path(
+ SerializePathForTesting(base::FilePath()));
+ EXPECT_FALSE(DeserializeAndResolveToken(data));
+ }
+#endif
+}
+
+TEST_F(FileSystemAccessManagerImplTest,
+ DeserializeHandle_Sandboxed_InvalidData) {
+ // `CreateCrackedFileSystemURL` for `kFileSystemTypeTemporary` does not reject
+ // `..` or `.` segments, so `DeserializeHandle` must reject them directly.
+ const base::FilePath invalid_virtual_paths[] = {
+ base::FilePath(FILE_PATH_LITERAL("../bar")),
+ base::FilePath(FILE_PATH_LITERAL(".")),
+ base::FilePath(FILE_PATH_LITERAL("./bar")),
+ base::FilePath(FILE_PATH_LITERAL("a/./b")),
+ };
+ for (const auto& virtual_path : invalid_virtual_paths) {
+ FileSystemAccessHandleData data;
+ data.set_handle_type(FileSystemAccessHandleData::kFile);
+ data.mutable_sandboxed()->set_virtual_path(
+ SerializePathForTesting(virtual_path));
+ EXPECT_FALSE(DeserializeAndResolveToken(data)) << virtual_path;
+ }
+
+ {
+ FileSystemAccessHandleData data;
+ data.set_handle_type(FileSystemAccessHandleData::kFile);
+ EXPECT_FALSE(DeserializeAndResolveToken(data));
+ }
+}
+
+#if BUILDFLAG(IS_ANDROID)
+TEST_F(FileSystemAccessManagerImplTest,
+ SerializeHandle_AndroidContentUri_FileInsideDirectory) {
+ const base::FilePath kRootTreeUri("content://authority/tree/foo");
+ const base::FilePath kChildDocUri(
+ "content://authority/tree/foo/document/doc%3Abar");
+ const PathInfo kRootPathInfo(kRootTreeUri, "foo_dir");
+
+ auto grant = base::MakeRefCounted<FixedFileSystemAccessPermissionGrant>(
+ FixedFileSystemAccessPermissionGrant::PermissionStatus::GRANTED,
+ kRootPathInfo);
+ storage::FileSystemURL child_url =
+ file_system_context_->CreateCrackedFileSystemURL(
+ blink::StorageKey(), storage::kFileSystemTypeLocal, kChildDocUri);
+ FileSystemAccessFileHandleImpl file(manager_.get(), binding_context_,
+ child_url, "bar", {grant, grant});
+ mojo::PendingRemote<blink::mojom::FileSystemAccessTransferToken> token_remote;
+ manager_->CreateTransferToken(file,
+ token_remote.InitWithNewPipeAndPassReceiver());
+
+ EXPECT_CALL(
+ permission_context_,
+ GetReadPermissionGrant(
+ kTestStorageKey.origin(), kRootPathInfo, HandleType::kDirectory,
+ FileSystemAccessPermissionContext::AccessTrigger::kLoadFromStorage))
+ .WillOnce(testing::Return(ask_grant_));
+ EXPECT_CALL(
+ permission_context_,
+ GetWritePermissionGrant(
+ kTestStorageKey.origin(), kRootPathInfo, HandleType::kDirectory,
+ FileSystemAccessPermissionContext::AccessTrigger::kLoadFromStorage))
+ .WillOnce(testing::Return(ask_grant2_));
+
+ FileSystemAccessTransferTokenImpl* token =
+ SerializeAndDeserializeToken(std::move(token_remote));
+ ASSERT_TRUE(token);
+ EXPECT_EQ(kChildDocUri, token->url().path());
+ EXPECT_EQ(HandleType::kFile, token->type());
+ EXPECT_EQ(ask_grant_, token->GetReadGrant());
+ EXPECT_EQ(ask_grant2_, token->GetWriteGrant());
+}
+#endif
+
// FileSystemAccessManager should successfully resolve a
// FileSystemAccessDataTransferToken representing a file in the user's file
// system into a valid Remote<blink::mojom::FileSystemAccessFileHandle>, given
diff --git a/content/test/BUILD.gn b/content/test/BUILD.gn
index c8c39658..3efda17 100644
--- a/content/test/BUILD.gn
+++ b/content/test/BUILD.gn
@@ -3315,6 +3315,7 @@
"//content/browser/cache_storage:cache_storage_proto",
"//content/browser/devtools:devtools_background_services_proto",
"//content/browser/download:proto",
... (file diff truncated)
Loading diff…
Regression Test / PoC
shipped with the fix
diff --git a/content/browser/file_system_access/file_system_access_manager_impl_unittest.cc b/content/browser/file_system_access/file_system_access_manager_impl_unittest.cc
index 4891438..e240e46a 100644
--- a/content/browser/file_system_access/file_system_access_manager_impl_unittest.cc
+++ b/content/browser/file_system_access/file_system_access_manager_impl_unittest.cc
@@ -31,6 +31,7 @@
#include "components/services/storage/public/cpp/buckets/constants.h"
#include "content/browser/blob_storage/chrome_blob_storage_context.h"
#include "content/browser/file_system_access/features.h"
+#include "content/browser/file_system_access/file_system_access.pb.h"
#include "content/browser/file_system_access/file_system_access_data_transfer_token_impl.h"
#include "content/browser/file_system_access/file_system_access_directory_handle_impl.h"
#include "content/browser/file_system_access/file_system_access_file_handle_impl.h"
@@ -413,6 +414,29 @@
return resolve_future.Get();
}
+ std::string SerializePathForTesting(const base::FilePath& path) {
+ auto path_bytes = base::as_byte_span(path.value());
+ return std::string(path_bytes.begin(), path_bytes.end());
+ }
+
+ FileSystemAccessTransferTokenImpl* DeserializeAndResolveToken(
+ const FileSystemAccessHandleData& data) {
+ std::string serialized_string;
+ EXPECT_TRUE(data.SerializeToString(&serialized_string));
+ std::vector<uint8_t> serialized(serialized_string.begin(),
+ serialized_string.end());
+ EXPECT_FALSE(serialized.empty());
+
+ mojo::PendingRemote<blink::mojom::FileSystemAccessTransferToken>
+ token_remote;
+ manager_->DeserializeHandle(kTestStorageKey, serialized,
+ token_remote.InitWithNewPipeAndPassReceiver());
+ base::test::TestFuture<FileSystemAccessTransferTokenImpl*> resolve_future;
+ manager_->ResolveTransferToken(std::move(token_remote),
+ resolve_future.GetCallback());
+ return resolve_future.Get();
+ }
+
void GetEntryFromDataTransferTokenFileTest(
const PathInfo& file_path_info,
const std::string& expected_file_contents) {
@@ -1822,6 +1846,183 @@
EXPECT_EQ(ask_grant2_, token->GetWriteGrant());
}
+TEST_F(FileSystemAccessManagerImplTest,
+ DeserializeHandle_Native_InvalidRelativePath) {
+ const base::FilePath root_path = dir_.GetPath().AppendASCII("foo");
+ const base::FilePath invalid_relative_paths[] = {
+ base::FilePath(FILE_PATH_LITERAL("..")),
+ base::FilePath(FILE_PATH_LITERAL("../bar")),
+ base::FilePath(FILE_PATH_LITERAL("../other_dir")),
+ base::FilePath(FILE_PATH_LITERAL("sub/../../bar")),
+ base::FilePath(FILE_PATH_LITERAL(".")),
+ base::FilePath(FILE_PATH_LITERAL("./bar")),
+ dir_.GetPath().AppendASCII("bar"),
+#if BUILDFLAG(IS_WIN)
+ base::FilePath(FILE_PATH_LITERAL("C:bar")),
+ base::FilePath(FILE_PATH_LITERAL("\\bar")),
+#endif
+ };
+
+ EXPECT_CALL(permission_context_, GetReadPermissionGrant).Times(0);
+ EXPECT_CALL(permission_context_, GetWritePermissionGrant).Times(0);
+
+ for (auto handle_type : {FileSystemAccessHandleData::kFile,
+ FileSystemAccessHandleData::kDirectory}) {
+ for (const auto& relative_path : invalid_relative_paths) {
+ FileSystemAccessHandleData data;
+ data.set_handle_type(handle_type);
+ data.mutable_local()->set_root_path(SerializePathForTesting(root_path));
+ data.mutable_local()->set_relative_path(
+ SerializePathForTesting(relative_path));
+ EXPECT_FALSE(DeserializeAndResolveToken(data)) << relative_path;
+ }
+ }
+}
+
+TEST_F(FileSystemAccessManagerImplTest,
+ DeserializeHandle_Native_InvalidRootPath) {
+ const base::FilePath invalid_root_paths[] = {
+ base::FilePath(),
+ dir_.GetPath().AppendASCII("foo").Append(FILE_PATH_LITERAL("../bar")),
+ base::FilePath(FILE_PATH_LITERAL("relative/root")),
+ };
+
+ EXPECT_CALL(permission_context_, GetReadPermissionGrant).Times(0);
+ EXPECT_CALL(permission_context_, GetWritePermissionGrant).Times(0);
+
+ for (const auto& root_path : invalid_root_paths) {
+ FileSystemAccessHandleData data;
+ data.set_handle_type(FileSystemAccessHandleData::kDirectory);
+ data.mutable_local()->set_root_path(SerializePathForTesting(root_path));
+ data.mutable_local()->set_relative_path(
+ SerializePathForTesting(base::FilePath()));
+ EXPECT_FALSE(DeserializeAndResolveToken(data)) << root_path;
+ }
+}
+
+TEST_F(FileSystemAccessManagerImplTest,
+ DeserializeHandle_External_InvalidPaths) {
+ EXPECT_CALL(permission_context_, GetReadPermissionGrant).Times(0);
+ EXPECT_CALL(permission_context_, GetWritePermissionGrant).Times(0);
+
+ {
+ FileSystemAccessHandleData data;
+ data.set_handle_type(FileSystemAccessHandleData::kFile);
+ data.mutable_external()->set_root_path(SerializePathForTesting(
+ base::FilePath::FromUTF8Unsafe(kTestMountPoint).AppendASCII("foo")));
+ data.mutable_external()->set_relative_path(
+ SerializePathForTesting(base::FilePath(FILE_PATH_LITERAL("../bar"))));
+ EXPECT_FALSE(DeserializeAndResolveToken(data));
+ }
+
+ {
+ FileSystemAccessHandleData data;
+ data.set_handle_type(FileSystemAccessHandleData::kFile);
+ data.mutable_external()->set_root_path(
+ SerializePathForTesting(base::FilePath::FromUTF8Unsafe(kTestMountPoint)
+ .Append(FILE_PATH_LITERAL("../foo"))));
+ data.mutable_external()->set_relative_path(
+ SerializePathForTesting(base::FilePath()));
+ EXPECT_FALSE(DeserializeAndResolveToken(data));
+ }
+
+ // Unregistered external mounts produce an invalid `root` URL. Without the
+ // `root.is_valid()` check, `CreateCrackedFileSystemURL` on
+ // `kFileSystemTypeUnknown` returns a URL with `is_valid() == true`.
+ {
+ FileSystemAccessHandleData data;
+ data.set_handle_type(FileSystemAccessHandleData::kFile);
+ data.mutable_external()->set_root_path(SerializePathForTesting(
+ base::FilePath::FromUTF8Unsafe("unregistered_mount")
+ .AppendASCII("foo")));
+ data.mutable_external()->set_relative_path(
+ SerializePathForTesting(base::FilePath()));
+ EXPECT_FALSE(DeserializeAndResolveToken(data));
+ }
+
+#if BUILDFLAG(IS_WIN)
+ // A bare drive letter passes `IsValidRootPath` for `kExternal`, but
+ // `FilePath::BaseName()` strips the drive letter and returns an empty string.
+ {
+ FileSystemAccessHandleData data;
+ data.set_handle_type(FileSystemAccessHandleData::kDirectory);
+ data.mutable_external()->set_root_path(
+ SerializePathForTesting(base::FilePath(FILE_PATH_LITERAL("C:"))));
+ data.mutable_external()->set_relative_path(
+ SerializePathForTesting(base::FilePath()));
+ EXPECT_FALSE(DeserializeAndResolveToken(data));
+ }
+#endif
+}
+
+TEST_F(FileSystemAccessManagerImplTest,
+ DeserializeHandle_Sandboxed_InvalidData) {
+ // `CreateCrackedFileSystemURL` for `kFileSystemTypeTemporary` does not reject
+ // `..` or `.` segments, so `DeserializeHandle` must reject them directly.
+ const base::FilePath invalid_virtual_paths[] = {
+ base::FilePath(FILE_PATH_LITERAL("../bar")),
+ base::FilePath(FILE_PATH_LITERAL(".")),
+ base::FilePath(FILE_PATH_LITERAL("./bar")),
+ base::FilePath(FILE_PATH_LITERAL("a/./b")),
+ };
+ for (const auto& virtual_path : invalid_virtual_paths) {
+ FileSystemAccessHandleData data;
+ data.set_handle_type(FileSystemAccessHandleData::kFile);
+ data.mutable_sandboxed()->set_virtual_path(
+ SerializePathForTesting(virtual_path));
+ EXPECT_FALSE(DeserializeAndResolveToken(data)) << virtual_path;
+ }
+
+ {
+ FileSystemAccessHandleData data;
+ data.set_handle_type(FileSystemAccessHandleData::kFile);
+ EXPECT_FALSE(DeserializeAndResolveToken(data));
+ }
+}
+
+#if BUILDFLAG(IS_ANDROID)
+TEST_F(FileSystemAccessManagerImplTest,
+ SerializeHandle_AndroidContentUri_FileInsideDirectory) {
+ const base::FilePath kRootTreeUri("content://authority/tree/foo");
+ const base::FilePath kChildDocUri(
+ "content://authority/tree/foo/document/doc%3Abar");
+ const PathInfo kRootPathInfo(kRootTreeUri, "foo_dir");
+
+ auto grant = base::MakeRefCounted<FixedFileSystemAccessPermissionGrant>(
+ FixedFileSystemAccessPermissionGrant::PermissionStatus::GRANTED,
+ kRootPathInfo);
+ storage::FileSystemURL child_url =
+ file_system_context_->CreateCrackedFileSystemURL(
+ blink::StorageKey(), storage::kFileSystemTypeLocal, kChildDocUri);
+ FileSystemAccessFileHandleImpl file(manager_.get(), binding_context_,
+ child_url, "bar", {grant, grant});
+ mojo::PendingRemote<blink::mojom::FileSystemAccessTransferToken> token_remote;
+ manager_->CreateTransferToken(file,
+ token_remote.InitWithNewPipeAndPassReceiver());
+
+ EXPECT_CALL(
+ permission_context_,
+ GetReadPermissionGrant(
+ kTestStorageKey.origin(), kRootPathInfo, HandleType::kDirectory,
+ FileSystemAccessPermissionContext::AccessTrigger::kLoadFromStorage))
+ .WillOnce(testing::Return(ask_grant_));
+ EXPECT_CALL(
+ permission_context_,
+ GetWritePermissionGrant(
+ kTestStorageKey.origin(), kRootPathInfo, HandleType::kDirectory,
+ FileSystemAccessPermissionContext::AccessTrigger::kLoadFromStorage))
+ .WillOnce(testing::Return(ask_grant2_));
+
+ FileSystemAccessTransferTokenImpl* token =
+ SerializeAndDeserializeToken(std::move(token_remote));
+ ASSERT_TRUE(token);
+ EXPECT_EQ(kChildDocUri, token->url().path());
+ EXPECT_EQ(HandleType::kFile, token->type());
+ EXPECT_EQ(ask_grant_, token->GetReadGrant());
+ EXPECT_EQ(ask_grant2_, token->GetWriteGrant());
+}
+#endif
+
// FileSystemAccessManager should successfully resolve a
// FileSystemAccessDataTransferToken representing a file in the user's file
// system into a valid Remote<blink::mojom::FileSystemAccessFileHandle>, given
diff --git a/content/test/BUILD.gn b/content/test/BUILD.gn
index c8c39658..3efda17 100644
--- a/content/test/BUILD.gn
+++ b/content/test/BUILD.gn
@@ -3315,6 +3315,7 @@
"//content/browser/cache_storage:cache_storage_proto",
"//content/browser/devtools:devtools_background_services_proto",
"//content/browser/download:proto",
+ "//content/browser/file_system_access:proto",
"//content/browser/indexed_db:unit_tests",
"//content/browser/notifications:notification_proto",
"//content/browser/tracing/traces_internals:mojo_bindings",
Loading diff…
Original Bug Report
The reporter's bug is still restricted on the tracker. Chrome de-restricts security bugs ~30–90 days after the fix ships; a later run will backfill it here.
References
On This Page