Skip to content

Commit

Permalink
[M110] FSA: Disable blocklist check on dropped files
Browse files Browse the repository at this point in the history 
Disables the logic added in https://crrev.com/c/3945614.
This additional check is breaking some applications

(cherry picked from commit ccdc5ab)

Bug: 1370433
Change-Id: Ica9ec240cf7be6ed39db1a87df89d8cf6e2687db
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4133554
Auto-Submit: Austin Sullivan <asully@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1089983}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4151977
Commit-Queue: Austin Sullivan <asully@chromium.org>
Reviewed-by: Daseul Lee <dslee@chromium.org>
Cr-Commit-Position: refs/branch-heads/5481@{#242}
Cr-Branched-From: 130f3e4-refs/heads/main@{#1084008}
  • Loading branch information
@a-sully
a-sully authored and Chromium LUCI CQ committed Jan 12, 2023
1 parent 624e6ae commit 25ba156
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 21 deletions.
8 changes: 8 additions & 0 deletions content/browser/file_system_access/features.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,14 @@

namespace content::features {

// TODO(crbug.com/1370433): Remove this flag eventually.
// When enabled, drag-and-dropped files and directories will be checked against
// the File System Access blocklist. This feature was disabled since it broke
// some applications.
BASE_FEATURE(kFileSystemAccessDragAndDropCheckBlocklist,
"FileSystemAccessDragAndDropCheckBlocklist",
base::FEATURE_DISABLED_BY_DEFAULT);

// TODO(crbug.com/1381621): Remove this flag eventually.
// When enabled, move() will result in a promise rejection when the specified
// destination to move to exists. This feature was disabled since it does not
Expand Down
1 change: 1 addition & 0 deletions content/browser/file_system_access/features.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ namespace content::features {
// alongside the definition of their values in the .cc file.

// Alphabetical:
CONTENT_EXPORT BASE_DECLARE_FEATURE(kFileSystemAccessDragAndDropCheckBlocklist);
CONTENT_EXPORT BASE_DECLARE_FEATURE(kFileSystemAccessDoNotOverwriteOnMove);
CONTENT_EXPORT BASE_DECLARE_FEATURE(kFileSystemAccessRemove);
CONTENT_EXPORT BASE_DECLARE_FEATURE(kFileSystemAccessRemoveEntryExclusiveLock);
Expand Down
5 changes: 4 additions & 1 deletion content/browser/file_system_access/file_system_access_manager_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include "build/build_config.h"
#include "components/services/storage/public/cpp/buckets/bucket_id.h"
#include "components/services/storage/public/cpp/buckets/bucket_locator.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_access_handle_host_impl.h"
#include "content/browser/file_system_access/file_system_access_data_transfer_token_impl.h"
Expand Down Expand Up @@ -672,7 +673,9 @@ void FileSystemAccessManagerImpl::ResolveDataTransferTokenWithFileType(
HandleType file_type) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);

if (!permission_context_) {
if (!permission_context_ ||
!base::FeatureList::IsEnabled(
features::kFileSystemAccessDragAndDropCheckBlocklist)) {
DidVerifySensitiveDirectoryAccessForDataTransfer(
binding_context, file_path, url, file_type,
std::move(token_resolved_callback), SensitiveEntryResult::kAllowed);
Expand Down
58 changes: 38 additions & 20 deletions content/browser/file_system_access/file_system_access_manager_impl_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include <vector>

#include "base/callback_helpers.h"
#include "base/feature_list.h"
#include "base/files/file_util.h"
#include "base/files/scoped_temp_dir.h"
#include "base/guid.h"
Expand All @@ -23,6 +24,7 @@
#include "components/services/storage/public/cpp/buckets/bucket_id.h"
#include "components/services/storage/public/cpp/buckets/bucket_locator.h"
#include "components/services/storage/public/cpp/buckets/constants.h"
#include "content/browser/file_system_access/features.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"
Expand Down Expand Up @@ -249,16 +251,19 @@ class FileSystemAccessManagerImplTest : public testing::Test {
path_type, file_path, kBindingContext.process_id(),
token_remote.InitWithNewPipeAndPassReceiver());

EXPECT_CALL(
permission_context_,
ConfirmSensitiveEntryAccess_(
kTestStorageKey.origin(),
FileSystemAccessPermissionContext::PathType::kLocal, file_path,
FileSystemAccessPermissionContext::HandleType::kFile,
FileSystemAccessPermissionContext::UserAction::kDragAndDrop,
kFrameId, testing::_))
.WillOnce(RunOnceCallback<6>(
FileSystemAccessPermissionContext::SensitiveEntryResult::kAllowed));
if (base::FeatureList::IsEnabled(
features::kFileSystemAccessDragAndDropCheckBlocklist)) {
EXPECT_CALL(
permission_context_,
ConfirmSensitiveEntryAccess_(
kTestStorageKey.origin(),
FileSystemAccessPermissionContext::PathType::kLocal, file_path,
FileSystemAccessPermissionContext::HandleType::kFile,
FileSystemAccessPermissionContext::UserAction::kDragAndDrop,
kFrameId, testing::_))
.WillOnce(RunOnceCallback<6>(FileSystemAccessPermissionContext::
SensitiveEntryResult::kAllowed));
}

// Expect permission requests when the token is sent to be redeemed.
EXPECT_CALL(
Expand Down Expand Up @@ -307,16 +312,19 @@ class FileSystemAccessManagerImplTest : public testing::Test {
path_type, dir_path, kBindingContext.process_id(),
token_remote.InitWithNewPipeAndPassReceiver());

EXPECT_CALL(
permission_context_,
ConfirmSensitiveEntryAccess_(
kTestStorageKey.origin(),
FileSystemAccessPermissionContext::PathType::kLocal, dir_path,
FileSystemAccessPermissionContext::HandleType::kDirectory,
FileSystemAccessPermissionContext::UserAction::kDragAndDrop,
kFrameId, testing::_))
.WillOnce(RunOnceCallback<6>(
FileSystemAccessPermissionContext::SensitiveEntryResult::kAllowed));
if (base::FeatureList::IsEnabled(
features::kFileSystemAccessDragAndDropCheckBlocklist)) {
EXPECT_CALL(
permission_context_,
ConfirmSensitiveEntryAccess_(
kTestStorageKey.origin(),
FileSystemAccessPermissionContext::PathType::kLocal, dir_path,
FileSystemAccessPermissionContext::HandleType::kDirectory,
FileSystemAccessPermissionContext::UserAction::kDragAndDrop,
kFrameId, testing::_))
.WillOnce(RunOnceCallback<6>(FileSystemAccessPermissionContext::
SensitiveEntryResult::kAllowed));
}

// Expect permission requests when the token is sent to be redeemed.
EXPECT_CALL(
Expand Down Expand Up @@ -1285,6 +1293,11 @@ TEST_F(FileSystemAccessManagerImplTest,

TEST_F(FileSystemAccessManagerImplTest,
GetEntryFromDataTransferToken_File_SensitivePath) {
if (!base::FeatureList::IsEnabled(
features::kFileSystemAccessDragAndDropCheckBlocklist)) {
return;
}

base::FilePath file_path = dir_.GetPath().AppendASCII("mr_file");
ASSERT_TRUE(base::CreateTemporaryFile(&file_path));

Expand Down Expand Up @@ -1320,6 +1333,11 @@ TEST_F(FileSystemAccessManagerImplTest,

TEST_F(FileSystemAccessManagerImplTest,
GetEntryFromDataTransferToken_Directory_SensitivePath) {
if (!base::FeatureList::IsEnabled(
features::kFileSystemAccessDragAndDropCheckBlocklist)) {
return;
}

const base::FilePath& kDirPath = dir_.GetPath().AppendASCII("mr_directory");
ASSERT_TRUE(base::CreateDirectory(kDirPath));

Expand Down

0 comments on commit 25ba156

Please sign in to comment.