High chrome Logic Error 🔧 Commit mapped

Overview

High
Severity
—
CVSS
No
Exploited ITW
Fixed
Fix Status
ImpactUse after free in Quota
DescriptionUse after free in Quota
ComponentQuota
Bug ClassLogic Error
Tracker575202553
Fix commit71e66ee87824 (chromium/src) +153/-7
CISA KEVNot listed
CreditedGoogle
Disclosed2026-10-29

Background

`StorageDirectory`
Browser-process helper (storage/browser/quota/storage_directory.cc) that manages the on-disk WebStorage bucket directories, including “dooming” (atomically renaming aside) and later clearing them.
`DoomPath()` / `DoomBucket()` / `Doom()`
“Doom” renames a live directory to a sibling <name>-doomed name so it can be deleted asynchronously by ClearDoomed*().
`base::Move` fallback
base::Move(src, dst) first tries rename(2); if that fails (e.g. EISDIR when dst already exists as a directory) it falls back to base::CopyDirectory, which follows symbolic links and copies the link target’s contents.
`base::ReplaceFile` (POSIX)
Performs a direct rename(2) with no copy fallback, so a symlink is renamed (not followed).
`base::FileEnumerator::DIRECTORIES` vs `NAMES_ONLY`
DIRECTORIES skips entries that are symlinks to files / nonexistent targets; NAMES_ONLY returns every matching name.
`QuotaDatabase::DeleteBucketData`
Dooms a bucket directory then deletes the DB row; early-returns on error.

Root Cause Analysis

StorageDirectory::DoomPath() pre-created the destination with base::CreateTemporaryDirInDir(...) and then called base::Move(path, doomed_dir). When path had been swapped for a symbolic link, rename(2) onto the already-existing doomed_dir directory fails with EISDIR, so base::Move fell back to the symlink-following base::CopyDirectory, copying the link target’s contents into the profile’s WebStorage area — i.e. the browser process reads/writes through an attacker-planted link to an external location. Two related defects compounded it: DoomPath guarded with base::PathExists(path) (which follows symlinks), so a dangling symlink returned early and was never removed; and EnumerateDoomedDirectories/EnumerateDoomedBuckets used base::FileEnumerator::DIRECTORIES, which skips doomed symlinks-to-files or broken links, so ClearDoomed*() never cleaned them up (leaving a persistent link and leaking the follow behavior). Separately, QuotaDatabase::DeleteBucketData early-returned on failure without calling ClearDoomedBuckets(), leaving doomed entries uncleared.

The fix rewrites DoomPath to rename path onto a fresh, non-pre-created <name>-doomed-<uuid> name: on POSIX via base::ReplaceFile (a direct rename(2) with no copy fallback — a real directory moves, a planted symlink is renamed without being followed and later unlinked), and on non-POSIX via base::DeletePathRecursively for links or base::Move onto the non-existent target. The existence guard now also checks base::IsLink(path) so dangling symlinks are doomed too. Both enumerators switch to NAMES_ONLY so any doomed entry (including symlinks) reaches base::DeletePathRecursively. DeleteBucketData now calls ClearDoomedBuckets() on its error-return paths. POSIX regression tests assert that dooming a bucket/storage symlink renames the link (not the target), clears it, and leaves the external target file untouched.

The advisory’s “Use after free in Quota” is not substantiated by the diff: there is no object-lifetime/freed-memory bug. The code change fixes a symlink-following file operation (via base::Move’s CopyDirectory fallback) and incomplete symlink cleanup/enumeration in the browser process — a filesystem/path-handling flaw. Noted as an advisory/diff discrepancy.

Key insight
Dooming a storage directory pre-created the destination and used base::Move, whose rename(2)→CopyDirectory fallback follows symlinks; a symlink planted where a bucket/storage directory was expected would be followed by the browser process, copying an external target into the profile, and broken/file symlinks escaped both the existence guard and the doomed-entry cleanup.

Attack Path

  1. Plant a symlink in WebStorage Attacker with write access to the profile’s WebStorage area replaces a bucket (or storage) directory path with a symbolic link to an external target (inferred precondition; the diff proves the following/cleanup behavior).
  2. Trigger a doom Normal operation dooms that path: DoomBucket/Doom → DoomPath → base::Move(path, doomed_dir) where doomed_dir was just pre-created as a directory.
  3. `rename(2)` fails, copy fallback follows the link EISDIR makes base::Move fall back to base::CopyDirectory, which resolves the symlink and copies the external target’s contents into the profile; dangling links instead escape via the PathExists guard and persist.
  4. Arbitrary read-into-profile / persistent link The browser process ingests attacker-chosen external content (information disclosure into the profile) and/or leaves an uncleared link, with browser-process privileges.

Impact Assessment

Browser-process filesystem flaw, not a renderer memory-safety bug. Exploitation requires the ability to plant symlinks under the profile’s WebStorage directory (local attacker or a prior foothold), so it is a local integrity/disclosure issue rather than remote RCE. The gained primitive is symlink-directed directory copy/read performed by the browser process during bucket/storage dooming, plus incomplete cleanup of planted links. It is not a use-after-free despite the advisory label, and not RCE; strength is bounded by what the browser-process identity can reach via the planted link.

Changed Functions

FunctionChangeNotes
if
storage/browser/quota/quota_database.cc
modified
TEST_F
storage/browser/quota/storage_directory_unittest.cc
modified

Files Changed

  • storage/browser/quota/storage_directory.cc
  • storage/browser/quota/quota_database_unittest.cc
  • storage/browser/quota/storage_directory_unittest.cc
  • storage/browser/quota/quota_database.cc

Audit Directions

  • `base::Move` copy fallback
    Audit callers that base::Move onto a pre-existing/pre-created destination where the source could be attacker-controlled; the CopyDirectory fallback follows symlinks.
  • Existence checks before deletion
    Replace base::PathExists-only guards with IsLink-aware checks wherever dangling symlinks must still be removed (companion CVE-2026-119265 is the same class in MoveLegacyDatabase).
  • FileEnumerator filters
    Review FileEnumerator::DIRECTORIES/FILES uses in cleanup code that must also catch symlinks (prefer NAMES_ONLY).
  • Error-path cleanup
    Check storage error returns (DeleteBucketData and peers) for leaked doomed directories not passed to ClearDoomed*().
From 71e66ee87824aafdddb32ec7971165f8aa7997cb Mon Sep 17 00:00:00 2001
From: Eriko Kurimoto <[email protected]>
Date: Thu, 22 Oct 2026 23:51:34 -0700
Subject: [PATCH] Avoid CopyDirectory fallback in StorageDirectory::DoomPath

Previously, StorageDirectory::DoomPath pre-created a temporary target
directory with base::CreateTemporaryDirInDir and then called
base::Move(path, doomed_dir). When `path` is a symbolic link, moving it
onto an existing directory fails `rename(2)` with EISDIR and falls back
to base::CopyDirectory, which follows the link.

In addition:
- base::PathExists follows symlinks, so a dangling symlink at `path`
  returned early from DoomPath without being removed.
- EnumerateDoomedDirectories and EnumerateDoomedBuckets used
  base::FileEnumerator::DIRECTORIES, skipping doomed symlinks to files
  or nonexistent targets.
- QuotaDatabase::DeleteBucketData returned early on failure without
  clearing doomed bucket directories.

This CL:
- Renames `path` onto a fresh `<name>-doomed-<uuid>` path without
  pre-creating the destination directory, using base::ReplaceFile on
  POSIX (so `rename(2)` does not fall back to CopyDirectory) and
  base::Move (after unlinking reparse points) on non-POSIX platforms,
  and checks base::IsLink so dangling symlinks are also doomed.
- Uses base::FileEnumerator::NAMES_ONLY in EnumerateDoomedDirectories
  and EnumerateDoomedBuckets so any doomed entry is passed to
  base::DeletePathRecursively.
- Calls ClearDoomedBuckets() on early-return error paths in
  QuotaDatabase::DeleteBucketData.

TAG=agy
CONV=b03dcafb-e302-4850-a029-78a6d8b7c1b2

Bug: 575202553
Change-Id: I575202553a7f4070f631e29a8b0ab2041285d940
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8603890
Auto-Submit: Eriko Kurimoto <[email protected]>
Reviewed-by: Ming-Ying Chung <[email protected]>
Commit-Queue: Ming-Ying Chung <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1725545}
---
diff --git a/storage/browser/quota/quota_database.cc b/storage/browser/quota/quota_database.cc
index 85e8da6..b4d082a 100644
--- a/storage/browser/quota/quota_database.cc
+++ b/storage/browser/quota/quota_database.cc
@@ -586,6 +586,7 @@
   // directory deletion fails. `storage_directory_` may be nullptr for
   // in-memory only.
   if (storage_directory_ && !storage_directory_->DoomBucket(bucket)) {
+    storage_directory_->ClearDoomedBuckets();
     return base::unexpected(QuotaError::kFileOperationError);
   }
 
@@ -596,6 +597,9 @@
   statement.BindInt64(0, bucket.id.value());
 
   if (!statement.Step()) {
+    if (storage_directory_) {
+      storage_directory_->ClearDoomedBuckets();
+    }
     return base::unexpected(QuotaError::kDatabaseError);
   }
 
diff --git a/storage/browser/quota/storage_directory.cc b/storage/browser/quota/storage_directory.cc
index cc7fc74..8d88c0b 100644
--- a/storage/browser/quota/storage_directory.cc
+++ b/storage/browser/quota/storage_directory.cc
@@ -8,6 +8,8 @@
 #include "base/files/file_util.h"
 #include "base/notreached.h"
 #include "base/strings/strcat.h"
+#include "base/uuid.h"
+#include "build/build_config.h"
 #include "components/services/storage/public/cpp/constants.h"
 #include "storage/browser/quota/storage_directory_util.h"
 
@@ -59,20 +61,43 @@
 }
 
 bool StorageDirectory::DoomPath(const base::FilePath& path) {
-  if (!base::PathExists(path))
+  if (!base::PathExists(path) && !base::IsLink(path)) {
     return true;
+  }
 
-  base::FilePath doomed_dir;
-  base::CreateTemporaryDirInDir(
-      path.DirName(), base::StrCat({path.BaseName().value(), kDoomedPathName}),
-      &doomed_dir);
+  // Atomically rename `path` onto a fresh name that does not exist yet.
+  //
+  // Do not pre-create the destination directory: when `rename(2)` fails (e.g.
+  // with `EISDIR` on POSIX because `path` has been swapped for a symlink and
+  // the destination already exists as a directory), `base::Move()` falls back
+  // to the symlink-following `base::CopyDirectory()` and copies the link target
+  // into the profile directory. On POSIX, `base::ReplaceFile()` performs a
+  // direct `rename(2)` without a copy fallback: a real directory moves as
+  // before, while a planted symlink is renamed without being followed and then
+  // unlinked by `ClearDoomed*()`. On Windows, `base::ReplaceFile()` creates a
+  // temporary backup file and calls Win32 `::ReplaceFile()` (which fails for
+  // directories and logs a failed `Windows.ReplaceFileResult` histogram sample
+  // before falling back to `::MoveFile()`), so remove reparse points directly
+  // and use `base::Move()` onto the non-existent `doomed_dir`.
+  base::FilePath doomed_dir = path.DirName().Append(base::StrCat(
+      {path.BaseName().value(), kDoomedPathName,
+       base::FilePath::FromASCII(
+           base::Uuid::GenerateRandomV4().AsLowercaseString())
+           .value()}));
+#if BUILDFLAG(IS_POSIX)
+  return base::ReplaceFile(path, doomed_dir, /*error=*/nullptr);
+#else
+  if (base::IsLink(path)) {
+    return base::DeletePathRecursively(path);
+  }
   return base::Move(path, doomed_dir);
+#endif
 }
 
 std::set<base::FilePath> StorageDirectory::EnumerateDoomedDirectories() {
   base::FileEnumerator enumerator(
       web_storage_path_.DirName(),
-      /*recursive=*/false, base::FileEnumerator::DIRECTORIES,
+      /*recursive=*/false, base::FileEnumerator::NAMES_ONLY,
       base::StrCat(
           {kWebStorageDirectory, kDoomedPathName, FILE_PATH_LITERAL("*")}));
 
@@ -85,7 +110,7 @@
 
 std::set<base::FilePath> StorageDirectory::EnumerateDoomedBuckets() {
   base::FileEnumerator enumerator(
-      web_storage_path_, /*recursive=*/false, base::FileEnumerator::DIRECTORIES,
+      web_storage_path_, /*recursive=*/false, base::FileEnumerator::NAMES_ONLY,
       base::StrCat(
           {FILE_PATH_LITERAL("*"), kDoomedPathName, FILE_PATH_LITERAL("*")}));
 
diff --git a/storage/browser/quota/quota_database_unittest.cc b/storage/browser/quota/quota_database_unittest.cc
index c47a3f8c..b15b33d8 100644
--- a/storage/browser/quota/quota_database_unittest.cc
+++ b/storage/browser/quota/quota_database_unittest.cc
@@ -15,6 +15,7 @@
 
 #include "base/compiler_specific.h"
 #include "base/containers/flat_map.h"
+#include "base/files/file_enumerator.h"
 #include "base/files/file_util.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/functional/bind.h"
@@ -814,6 +815,19 @@
 
     ASSERT_TRUE(db->DeleteBucketData(bucket).has_value());
     ASSERT_FALSE(base::PathExists(bucket_path));
+
+    // Deleting a bucket that is missing from the database should still clean up
+    // its doomed directory on the error path.
+    ASSERT_TRUE(base::CreateDirectory(bucket_path));
+    ASSERT_TRUE(
+        base::WriteFile(bucket_path.AppendASCII("orphan"), "orphan_data"));
+    EXPECT_THAT(db->DeleteBucketData(bucket),
+                base::test::ErrorIs(QuotaError::kDatabaseError));
+    EXPECT_FALSE(base::PathExists(bucket_path));
+    base::FileEnumerator enumerator(
+        db->GetStoragePath(), /*recursive=*/false,
+        base::FileEnumerator::NAMES_ONLY, FILE_PATH_LITERAL("*-doomed-*"));
+    EXPECT_TRUE(enumerator.Next().empty());
   }
 }
 
diff --git a/storage/browser/quota/storage_directory_unittest.cc b/storage/browser/quota/storage_directory_unittest.cc
index 7d727ea3..9337e47 100644
--- a/storage/browser/quota/storage_directory_unittest.cc
+++ b/storage/browser/quota/storage_directory_unittest.cc
@@ -9,6 +9,7 @@
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/files/scoped_temp_dir.h"
+#include "build/build_config.h"
 #include "components/services/storage/public/cpp/buckets/bucket_locator.h"
 #include "storage/browser/quota/quota_client_type.h"
 #include "storage/browser/quota/storage_directory.h"
@@ -168,4 +169,106 @@
   EXPECT_EQ(directories.size(), 0u);
 }
 
+#if BUILDFLAG(IS_POSIX)
+TEST_F(StorageDirectoryTest, DoomAndClearBucketSymlinkDoesNotFollowTarget) {
+  base::FilePath external_dir =
+      temp_directory_.GetPath().AppendASCII("external_target");
+  ASSERT_TRUE(base::CreateDirectory(external_dir));
+  base::FilePath secret_file = external_dir.AppendASCII("secret.txt");
+  ASSERT_TRUE(base::WriteFile(secret_file, "secret_content"));
+
+  ASSERT_TRUE(storage_directory_->Create());
+
+  BucketLocator bucket(
+      BucketId(1),
+      blink::StorageKey::CreateFromStringForTesting("http://example/"),
+      /*is_default=*/false);
+  base::FilePath bucket_path =
+      CreateBucketPath(storage_directory_->path().DirName(), bucket);
+  ASSERT_TRUE(base::CreateSymbolicLink(external_dir, bucket_path));
+
+  ASSERT_TRUE(storage_directory_->DoomBucket(bucket));
+  EXPECT_FALSE(base::PathExists(bucket_path));
+  EXPECT_FALSE(base::IsLink(bucket_path));
+
+  std::set<base::FilePath> doomed_buckets =
+      storage_directory_->EnumerateDoomedBucketsForTesting();
+  ASSERT_EQ(doomed_buckets.size(), 1u);
+  // The doomed entry must be the renamed symlink itself, not a copied
+  // directory.
+  EXPECT_TRUE(base::IsLink(*doomed_buckets.begin()));
+
+  storage_directory_->ClearDoomedBuckets();
+  EXPECT_TRUE(storage_directory_->EnumerateDoomedBucketsForTesting().empty());
+
+  // The external target directory and file must remain untouched.
+  std::string contents;
+  EXPECT_TRUE(base::ReadFileToString(secret_file, &contents));
+  EXPECT_EQ(contents, "secret_content");
+}
+
+TEST_F(StorageDirectoryTest, DoomAndClearBucketFileAndBrokenSymlinks) {
+  base::FilePath external_file =
+      temp_directory_.GetPath().AppendASCII("external_file.txt");
+  ASSERT_TRUE(base::WriteFile(external_file, "file_content"));
+  base::FilePath nonexistent_target =
+      temp_directory_.GetPath().AppendASCII("nonexistent");
+
+  ASSERT_TRUE(storage_directory_->Create());
+
+  BucketLocator bucket1(
+      BucketId(1),
+      blink::StorageKey::CreateFromStringForTesting("http://example/"),
+      /*is_default=*/false);
+  BucketLocator bucket2(
+      BucketId(2),
+      blink::StorageKey::CreateFromStringForTesting("http://example/"),
+      /*is_default=*/false);
+
+  base::FilePath bucket1_path =
+      CreateBucketPath(storage_directory_->path().DirName(), bucket1);
+  base::FilePath bucket2_path =
+      CreateBucketPath(storage_directory_->path().DirName(), bucket2);
+  ASSERT_TRUE(base::CreateSymbolicLink(external_file, bucket1_path));
+  ASSERT_TRUE(base::CreateSymbolicLink(nonexistent_target, bucket2_path));
+
+  ASSERT_TRUE(storage_directory_->DoomBucket(bucket1));
+  ASSERT_TRUE(storage_directory_->DoomBucket(bucket2));
+  EXPECT_FALSE(base::IsLink(bucket1_path));
+  EXPECT_FALSE(base::IsLink(bucket2_path));
+
+  std::set<base::FilePath> doomed_buckets =
+      storage_directory_->EnumerateDoomedBucketsForTesting();
+  EXPECT_EQ(doomed_buckets.size(), 2u);
+
+  storage_directory_->ClearDoomedBuckets();
+  EXPECT_TRUE(storage_directory_->EnumerateDoomedBucketsForTesting().empty());
+  EXPECT_TRUE(base::PathExists(external_file));
+}
+
+TEST_F(StorageDirectoryTest, DoomAndClearStorageSymlinkDoesNotFollowTarget) {
+  base::FilePath external_dir =
+      temp_directory_.GetPath().AppendASCII("external_target");
+  ASSERT_TRUE(base::CreateDirectory(external_dir));
+  base::FilePath secret_file = external_dir.AppendASCII("secret.txt");
+  ASSERT_TRUE(base::WriteFile(secret_file, "secret_content"));
+
+  ASSERT_TRUE(
+      base::CreateSymbolicLink(external_dir, storage_directory_->path()));
+
+  ASSERT_TRUE(storage_directory_->Doom());
+  EXPECT_FALSE(base::IsLink(storage_directory_->path()));
+
+  std::set<base::FilePath> doomed_dirs =
+      storage_directory_->EnumerateDoomedDirectoriesForTesting();
+  ASSERT_EQ(doomed_dirs.size(), 1u);
+  EXPECT_TRUE(base::IsLink(*doomed_dirs.begin()));
+
+  storage_directory_->ClearDoomed();
+  EXPECT_TRUE(
+      storage_directory_->EnumerateDoomedDirectoriesForTesting().empty());
+  EXPECT_TRUE(base::PathExists(secret_file));
+}
+#endif  // BUILDFLAG(IS_POSIX)
+
 }  // namespace storage
Loading diff…

Regression Test / PoC

shipped with the fix
diff --git a/storage/browser/quota/quota_database_unittest.cc b/storage/browser/quota/quota_database_unittest.cc
index c47a3f8c..b15b33d8 100644
--- a/storage/browser/quota/quota_database_unittest.cc
+++ b/storage/browser/quota/quota_database_unittest.cc
@@ -15,6 +15,7 @@
 
 #include "base/compiler_specific.h"
 #include "base/containers/flat_map.h"
+#include "base/files/file_enumerator.h"
 #include "base/files/file_util.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/functional/bind.h"
@@ -814,6 +815,19 @@
 
     ASSERT_TRUE(db->DeleteBucketData(bucket).has_value());
     ASSERT_FALSE(base::PathExists(bucket_path));
+
+    // Deleting a bucket that is missing from the database should still clean up
+    // its doomed directory on the error path.
+    ASSERT_TRUE(base::CreateDirectory(bucket_path));
+    ASSERT_TRUE(
+        base::WriteFile(bucket_path.AppendASCII("orphan"), "orphan_data"));
+    EXPECT_THAT(db->DeleteBucketData(bucket),
+                base::test::ErrorIs(QuotaError::kDatabaseError));
+    EXPECT_FALSE(base::PathExists(bucket_path));
+    base::FileEnumerator enumerator(
+        db->GetStoragePath(), /*recursive=*/false,
+        base::FileEnumerator::NAMES_ONLY, FILE_PATH_LITERAL("*-doomed-*"));
+    EXPECT_TRUE(enumerator.Next().empty());
   }
 }
diff --git a/storage/browser/quota/storage_directory_unittest.cc b/storage/browser/quota/storage_directory_unittest.cc
index 7d727ea3..9337e47 100644
--- a/storage/browser/quota/storage_directory_unittest.cc
+++ b/storage/browser/quota/storage_directory_unittest.cc
@@ -9,6 +9,7 @@
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/files/scoped_temp_dir.h"
+#include "build/build_config.h"
 #include "components/services/storage/public/cpp/buckets/bucket_locator.h"
 #include "storage/browser/quota/quota_client_type.h"
 #include "storage/browser/quota/storage_directory.h"
@@ -168,4 +169,106 @@
   EXPECT_EQ(directories.size(), 0u);
 }
 
+#if BUILDFLAG(IS_POSIX)
+TEST_F(StorageDirectoryTest, DoomAndClearBucketSymlinkDoesNotFollowTarget) {
+  base::FilePath external_dir =
+      temp_directory_.GetPath().AppendASCII("external_target");
+  ASSERT_TRUE(base::CreateDirectory(external_dir));
+  base::FilePath secret_file = external_dir.AppendASCII("secret.txt");
+  ASSERT_TRUE(base::WriteFile(secret_file, "secret_content"));
+
+  ASSERT_TRUE(storage_directory_->Create());
+
+  BucketLocator bucket(
+      BucketId(1),
+      blink::StorageKey::CreateFromStringForTesting("http://example/"),
+      /*is_default=*/false);
+  base::FilePath bucket_path =
+      CreateBucketPath(storage_directory_->path().DirName(), bucket);
+  ASSERT_TRUE(base::CreateSymbolicLink(external_dir, bucket_path));
+
+  ASSERT_TRUE(storage_directory_->DoomBucket(bucket));
+  EXPECT_FALSE(base::PathExists(bucket_path));
+  EXPECT_FALSE(base::IsLink(bucket_path));
+
+  std::set<base::FilePath> doomed_buckets =
+      storage_directory_->EnumerateDoomedBucketsForTesting();
+  ASSERT_EQ(doomed_buckets.size(), 1u);
+  // The doomed entry must be the renamed symlink itself, not a copied
+  // directory.
+  EXPECT_TRUE(base::IsLink(*doomed_buckets.begin()));
+
+  storage_directory_->ClearDoomedBuckets();
+  EXPECT_TRUE(storage_directory_->EnumerateDoomedBucketsForTesting().empty());
+
+  // The external target directory and file must remain untouched.
+  std::string contents;
+  EXPECT_TRUE(base::ReadFileToString(secret_file, &contents));
+  EXPECT_EQ(contents, "secret_content");
+}
+
+TEST_F(StorageDirectoryTest, DoomAndClearBucketFileAndBrokenSymlinks) {
+  base::FilePath external_file =
+      temp_directory_.GetPath().AppendASCII("external_file.txt");
+  ASSERT_TRUE(base::WriteFile(external_file, "file_content"));
+  base::FilePath nonexistent_target =
+      temp_directory_.GetPath().AppendASCII("nonexistent");
+
+  ASSERT_TRUE(storage_directory_->Create());
+
+  BucketLocator bucket1(
+      BucketId(1),
+      blink::StorageKey::CreateFromStringForTesting("http://example/"),
+      /*is_default=*/false);
+  BucketLocator bucket2(
+      BucketId(2),
+      blink::StorageKey::CreateFromStringForTesting("http://example/"),
+      /*is_default=*/false);
+
+  base::FilePath bucket1_path =
+      CreateBucketPath(storage_directory_->path().DirName(), bucket1);
+  base::FilePath bucket2_path =
+      CreateBucketPath(storage_directory_->path().DirName(), bucket2);
+  ASSERT_TRUE(base::CreateSymbolicLink(external_file, bucket1_path));
+  ASSERT_TRUE(base::CreateSymbolicLink(nonexistent_target, bucket2_path));
+
+  ASSERT_TRUE(storage_directory_->DoomBucket(bucket1));
+  ASSERT_TRUE(storage_directory_->DoomBucket(bucket2));
+  EXPECT_FALSE(base::IsLink(bucket1_path));
+  EXPECT_FALSE(base::IsLink(bucket2_path));
+
+  std::set<base::FilePath> doomed_buckets =
+      storage_directory_->EnumerateDoomedBucketsForTesting();
+  EXPECT_EQ(doomed_buckets.size(), 2u);
+
+  storage_directory_->ClearDoomedBuckets();
+  EXPECT_TRUE(storage_directory_->EnumerateDoomedBucketsForTesting().empty());
+  EXPECT_TRUE(base::PathExists(external_file));
+}
+
+TEST_F(StorageDirectoryTest, DoomAndClearStorageSymlinkDoesNotFollowTarget) {
+  base::FilePath external_dir =
+      temp_directory_.GetPath().AppendASCII("external_target");
+  ASSERT_TRUE(base::CreateDirectory(external_dir));
+  base::FilePath secret_file = external_dir.AppendASCII("secret.txt");
+  ASSERT_TRUE(base::WriteFile(secret_file, "secret_content"));
+
+  ASSERT_TRUE(
+      base::CreateSymbolicLink(external_dir, storage_directory_->path()));
+
+  ASSERT_TRUE(storage_directory_->Doom());
+  EXPECT_FALSE(base::IsLink(storage_directory_->path()));
+
+  std::set<base::FilePath> doomed_dirs =
+      storage_directory_->EnumerateDoomedDirectoriesForTesting();
+  ASSERT_EQ(doomed_dirs.size(), 1u);
+  EXPECT_TRUE(base::IsLink(*doomed_dirs.begin()));
+
+  storage_directory_->ClearDoomed();
+  EXPECT_TRUE(
+      storage_directory_->EnumerateDoomedDirectoriesForTesting().empty());
+  EXPECT_TRUE(base::PathExists(secret_file));
+}
+#endif  // BUILDFLAG(IS_POSIX)
+
 }  // namespace storage
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.