Native FS: Refine restricted path blocking logic

The list of blocked paths may include nested entries.
Apply the child-blocking setting of the narrowest ancestor path.

Eg. If /foo blocks children, but /foo/bar doesn't, allow foo/bar/baz.

Bug: 996230
Test: Chrome OS allows Native FS access to files in Downloads, etc.
Change-Id: I4a0132f30c85328f543c6758aa76e1625dd21515
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1777102
Commit-Queue: Michael Wasserman <msw@chromium.org>
Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
Auto-Submit: Michael Wasserman <msw@chromium.org>
Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#691905}

chrome/browser/native_file_system/chrome_native_file_system_permission_context.cc

@@ -145,7 +145,10 @@ const struct {
 
   // If this is set to true not only is the given path blocked, all the children
   // are blocked as well. When this is set to false only the path and its
-  // parents are blocked.
+  // parents are blocked. 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 should *not* be blocked.
   bool block_all_children;
 } kBlockedPaths[] = {
     // Don't allow users to share their entire home directory, entire desktop or
@@ -203,6 +206,9 @@ const struct {
 bool ShouldBlockAccessToPath(const base::FilePath& check_path) {
   DCHECK(!check_path.empty());
   DCHECK(check_path.IsAbsolute());
+
+  base::FilePath nearest_ancestor;
+  bool nearest_ancestor_blocks_all_children = false;
   for (const auto& block : kBlockedPaths) {
     base::FilePath blocked_path;
     if (block.base_path_key != kNoBasePathKey) {
@@ -218,10 +224,14 @@ bool ShouldBlockAccessToPath(const base::FilePath& check_path) {
     if (check_path == blocked_path || check_path.IsParent(blocked_path))
       return true;
 
-    if (block.block_all_children && blocked_path.IsParent(check_path))
-      return true;
+    if (blocked_path.IsParent(check_path) &&
+        (nearest_ancestor.empty() || nearest_ancestor.IsParent(blocked_path))) {
+      nearest_ancestor = blocked_path;
+      nearest_ancestor_blocks_all_children = block.block_all_children;
     }
-  return false;
+  }
+
+  return !nearest_ancestor.empty() && nearest_ancestor_blocks_all_children;
 }
 
 // Returns a callback that calls the passed in |callback| by posting a task to

chrome/browser/native_file_system/chrome_native_file_system_permission_context_unittest.cc

@@ -364,6 +364,33 @@ TEST_F(ChromeNativeFileSystemPermissionContextTest,
                                                 {app_dir.AppendASCII("foo")}));
 }
 
+TEST_F(ChromeNativeFileSystemPermissionContextTest,
+       ConfirmSensitiveDirectoryAccess_BlockChildrenNested) {
+  base::FilePath user_data_dir = temp_dir_.GetPath().AppendASCII("user");
+  base::ScopedPathOverride user_data_override(chrome::DIR_USER_DATA,
+                                              user_data_dir, true, true);
+  base::FilePath download_dir = user_data_dir.AppendASCII("downloads");
+  base::ScopedPathOverride download_override(chrome::DIR_DEFAULT_DOWNLOADS,
+                                             download_dir, true, true);
+
+  // User Data directory itself should not be allowed.
+  EXPECT_EQ(SensitiveDirectoryResult::kAbort,
+            ConfirmSensitiveDirectoryAccessSync(permission_context(),
+                                                {user_data_dir}));
+  // Parent of User Data directory should also not be allowed.
+  EXPECT_EQ(SensitiveDirectoryResult::kAbort,
+            ConfirmSensitiveDirectoryAccessSync(permission_context(),
+                                                {temp_dir_.GetPath()}));
+  // The nested Download directory itself should not be allowed.
+  EXPECT_EQ(SensitiveDirectoryResult::kAbort,
+            ConfirmSensitiveDirectoryAccessSync(permission_context(),
+                                                {download_dir}));
+  // Paths inside the nested Download directory should be allowed.
+  EXPECT_EQ(SensitiveDirectoryResult::kAllowed,
+            ConfirmSensitiveDirectoryAccessSync(
+                permission_context(), {download_dir.AppendASCII("foo")}));
+}
+
 TEST_F(ChromeNativeFileSystemPermissionContextTest,
        ConfirmSensitiveDirectoryAccess_RelativePathBlock) {
   base::FilePath home_dir = temp_dir_.GetPath().AppendASCII("home");