Chrome · DOMStorage
CVE-2026-17933
Logic Error in DOMStorage
Overview
Low
Severity
—
CVSS
No
Exploited ITW
Fixed
Fix Status
Changed Functions
| Function | Change | Notes |
|---|---|---|
ifcomponents/services/storage/dom_storage/dom_storage_database.cc |
modified | |
TEST_Fcomponents/services/storage/dom_storage/dom_storage_database_unittest.cc |
modified | |
TEST_Fcomponents/services/storage/dom_storage/leveldb/local_storage_leveldb_unittest.cc |
modified |
Files Changed
components/services/storage/dom_storage/async_dom_storage_database_unittest.cccomponents/services/storage/dom_storage/dom_storage_database.cccomponents/services/storage/dom_storage/dom_storage_database.hcomponents/services/storage/dom_storage/dom_storage_database_unittest.cccomponents/services/storage/dom_storage/leveldb/local_storage_leveldb.cccomponents/services/storage/dom_storage/leveldb/local_storage_leveldb_unittest.cc
Patch
From c10fa161e693f671100982a1cb9b535361657b8c Mon Sep 17 00:00:00 2001
From: Steve Becker <stevebe@microsoft.com>
Date: Fri, 26 Jun 2026 01:15:28 -0700
Subject: [PATCH] [DomStorage] Don't persist next map ID
Deletes all code responsible for reading, writing and testing the next
map ID. There's no reason to store the next map ID in the database. On
first load, session storage retrieves all metadata from the database,
using it to calculate the next available map ID in
`SessionStorageMetadata::Initialize()`.
Bug: 513822044
Change-Id: I334763148e58fb6f4967a55caa4fa3a498f8694f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7997278
Reviewed-by: Rahul Singh <rahsin@microsoft.com>
Commit-Queue: Steve Becker <stevebe@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#1652969}
---
diff --git a/components/services/storage/dom_storage/async_dom_storage_database_unittest.cc b/components/services/storage/dom_storage/async_dom_storage_database_unittest.cc
index 270f572..7a3e9f8 100644
--- a/components/services/storage/dom_storage/async_dom_storage_database_unittest.cc
+++ b/components/services/storage/dom_storage/async_dom_storage_database_unittest.cc
@@ -144,9 +144,6 @@
ExpectEqualsMapMetadataSpan(read_metadata.map_metadata,
expected_map_metadata);
-
- // Local storage does not store the next map id number.
- EXPECT_EQ(read_metadata.next_map_id, std::nullopt);
}
// Delete the first and third storage keys.
@@ -160,7 +157,6 @@
DomStorageDatabase::Metadata read_metadata;
ASSERT_NO_FATAL_FAILURE(ReadAllMetadataSync(*database, &read_metadata));
- EXPECT_EQ(read_metadata.next_map_id, std::nullopt);
// Add the second and fourth storage keys as expected.
std::vector<DomStorageDatabase::MapMetadata> expected_metadata_after_delete;
diff --git a/components/services/storage/dom_storage/dom_storage_database.cc b/components/services/storage/dom_storage/dom_storage_database.cc
index e6da15b4..e8536cfe 100644
--- a/components/services/storage/dom_storage/dom_storage_database.cc
+++ b/components/services/storage/dom_storage/dom_storage_database.cc
@@ -609,13 +609,6 @@
ASSIGN_OR_RETURN(DomStorageDatabase::Metadata source_metadata,
source.ReadAllMetadata());
- // Migrate the `next_map_id` metadata.
- if (source_metadata.next_map_id) {
- DomStorageDatabase::Metadata map_id_metadata;
- map_id_metadata.next_map_id = source_metadata.next_map_id;
- destination.PutMetadata(std::move(map_id_metadata));
- }
-
// Migrate each map in `source_metadata`.
for (DomStorageDatabase::MapMetadata& source_map :
source_metadata.map_metadata) {
diff --git a/components/services/storage/dom_storage/dom_storage_database.h b/components/services/storage/dom_storage/dom_storage_database.h
index 9f6064a9..fb90bccf 100644
--- a/components/services/storage/dom_storage/dom_storage_database.h
+++ b/components/services/storage/dom_storage/dom_storage_database.h
@@ -181,7 +181,6 @@
Metadata& operator=(const Metadata&) = delete;
std::vector<MapMetadata> map_metadata;
- std::optional<int64_t> next_map_id;
};
// A collection of key/value pair updates for a single map. Optionally
diff --git a/components/services/storage/dom_storage/dom_storage_database_unittest.cc b/components/services/storage/dom_storage/dom_storage_database_unittest.cc
index 379b0b9a..2fac576 100644
--- a/components/services/storage/dom_storage/dom_storage_database_unittest.cc
+++ b/components/services/storage/dom_storage/dom_storage_database_unittest.cc
@@ -33,7 +33,6 @@
constexpr int64_t kFirstMapId = 10;
constexpr int64_t kSecondMapId = 11;
-constexpr int64_t kNextMapId = 12;
constexpr base::ByteSize kMapTotalSize{312};
@@ -116,7 +115,6 @@
ASSERT_OK_AND_ASSIGN(DomStorageDatabase::Metadata metadata,
destination->ReadAllMetadata());
EXPECT_TRUE(metadata.map_metadata.empty());
- EXPECT_EQ(metadata.next_map_id, std::nullopt);
}
TEST_F(DomStorageDatabaseTest, MigrateLocalStorageWithSingleMap) {
@@ -164,7 +162,6 @@
},
};
ExpectEqualsMapMetadataSpan(metadata.map_metadata, kExpectedMapMetadata);
- EXPECT_EQ(metadata.next_map_id, std::nullopt);
}
TEST_F(DomStorageDatabaseTest, MigrateLocalStorageWithMultipleMaps) {
@@ -221,8 +218,6 @@
// Verify metadata for both maps.
ASSERT_OK_AND_ASSIGN(DomStorageDatabase::Metadata metadata,
destination->ReadAllMetadata());
-
- EXPECT_EQ(metadata.next_map_id, std::nullopt);
ASSERT_EQ(metadata.map_metadata.size(), 2u);
// Each map must have a unique ID.
@@ -298,7 +293,6 @@
};
DomStorageDatabase::Metadata metadata;
- metadata.next_map_id = kNextMapId;
metadata.map_metadata = CloneMapMetadataVector(kExpectedMapMetadata);
DbStatus status = source->PutMetadata(std::move(metadata));
@@ -328,8 +322,6 @@
// Verify metadata was migrated.
ASSERT_OK_AND_ASSIGN(DomStorageDatabase::Metadata dest_metadata,
destination->ReadAllMetadata());
-
- EXPECT_EQ(dest_metadata.next_map_id, kNextMapId);
ExpectEqualsMapMetadataSpan(dest_metadata.map_metadata, kExpectedMapMetadata);
}
@@ -345,7 +337,6 @@
expected_map_metadata[0].map_locator.AddSession(kSecondSessionId);
DomStorageDatabase::Metadata metadata;
- metadata.next_map_id = kNextMapId;
metadata.map_metadata = CloneMapMetadataVector(expected_map_metadata);
DbStatus status = source->PutMetadata(std::move(metadata));
@@ -376,8 +367,6 @@
// Verify the cloned map's metadata migrated.
ASSERT_OK_AND_ASSIGN(DomStorageDatabase::Metadata dest_metadata,
destination->ReadAllMetadata());
-
- EXPECT_EQ(dest_metadata.next_map_id, kNextMapId);
ExpectEqualsMapMetadataSpan(dest_metadata.map_metadata,
expected_map_metadata);
}
@@ -399,7 +388,6 @@
};
DomStorageDatabase::Metadata metadata;
- metadata.next_map_id = kNextMapId;
metadata.map_metadata = CloneMapMetadataVector(kExpectedMapMetadata);
DbStatus status = source->PutMetadata(std::move(metadata));
@@ -444,8 +432,6 @@
// Verify metadata for both maps.
ASSERT_OK_AND_ASSIGN(DomStorageDatabase::Metadata dest_metadata,
destination->ReadAllMetadata());
-
- EXPECT_EQ(dest_metadata.next_map_id, kNextMapId);
ExpectEqualsMapMetadataSpan(dest_metadata.map_metadata, kExpectedMapMetadata);
}
diff --git a/components/services/storage/dom_storage/leveldb/local_storage_leveldb.cc b/components/services/storage/dom_storage/leveldb/local_storage_leveldb.cc
index e5941bee..462e6e3 100644
--- a/components/services/storage/dom_storage/leveldb/local_storage_leveldb.cc
+++ b/components/services/storage/dom_storage/leveldb/local_storage_leveldb.cc
@@ -290,9 +290,6 @@
}
DbStatus LocalStorageLevelDB::PutMetadata(Metadata metadata) {
- // Local storage does not record the next map id in LevelDB.
- CHECK(!metadata.next_map_id);
-
std::unique_ptr<DomStorageBatchOperationLevelDB> batch =
leveldb_->CreateBatchOperation();
diff --git a/components/services/storage/dom_storage/leveldb/local_storage_leveldb_unittest.cc b/components/services/storage/dom_storage/leveldb/local_storage_leveldb_unittest.cc
index c6aaf2f3..878e551 100644
--- a/components/services/storage/dom_storage/leveldb/local_storage_leveldb_unittest.cc
+++ b/components/services/storage/dom_storage/leveldb/local_storage_leveldb_unittest.cc
@@ -167,7 +167,6 @@
// Read back the map usage metadata from the database.
ASSERT_OK_AND_ASSIGN(DomStorageDatabase::Metadata all_metadata,
database.ReadAllMetadata());
- EXPECT_EQ(all_metadata.next_map_id, std::nullopt);
ExpectEqualsMapMetadataSpan(all_metadata.map_metadata,
base::span_from_ref(metadata_to_update));
}
@@ -338,7 +337,6 @@
ASSERT_OK_AND_ASSIGN(DomStorageDatabase::Metadata all_metadata,
local_storage_leveldb->ReadAllMetadata());
EXPECT_EQ(all_metadata.map_metadata.size(), 0u);
- EXPECT_EQ(all_metadata.next_map_id, std::nullopt);
}
TEST_F(LocalStorageLevelDBTest, ReadAllMetadataWithInvalid) {
@@ -356,7 +354,6 @@
ASSERT_OK_AND_ASSIGN(DomStorageDatabase::Metadata all_metadata,
local_storage_leveldb->ReadAllMetadata());
EXPECT_EQ(all_metadata.map_metadata.size(), 0u);
- EXPECT_EQ(all_metadata.next_map_id, std::nullopt);
}
Loading diff…
Regression Test / PoC
shipped with the fix
diff --git a/components/services/storage/dom_storage/async_dom_storage_database_unittest.cc b/components/services/storage/dom_storage/async_dom_storage_database_unittest.cc
index 270f572..7a3e9f8 100644
--- a/components/services/storage/dom_storage/async_dom_storage_database_unittest.cc
+++ b/components/services/storage/dom_storage/async_dom_storage_database_unittest.cc
@@ -144,9 +144,6 @@
ExpectEqualsMapMetadataSpan(read_metadata.map_metadata,
expected_map_metadata);
-
- // Local storage does not store the next map id number.
- EXPECT_EQ(read_metadata.next_map_id, std::nullopt);
}
// Delete the first and third storage keys.
@@ -160,7 +157,6 @@
DomStorageDatabase::Metadata read_metadata;
ASSERT_NO_FATAL_FAILURE(ReadAllMetadataSync(*database, &read_metadata));
- EXPECT_EQ(read_metadata.next_map_id, std::nullopt);
// Add the second and fourth storage keys as expected.
std::vector<DomStorageDatabase::MapMetadata> expected_metadata_after_delete;
diff --git a/components/services/storage/dom_storage/dom_storage_database_unittest.cc b/components/services/storage/dom_storage/dom_storage_database_unittest.cc
index 379b0b9a..2fac576 100644
--- a/components/services/storage/dom_storage/dom_storage_database_unittest.cc
+++ b/components/services/storage/dom_storage/dom_storage_database_unittest.cc
@@ -33,7 +33,6 @@
constexpr int64_t kFirstMapId = 10;
constexpr int64_t kSecondMapId = 11;
-constexpr int64_t kNextMapId = 12;
constexpr base::ByteSize kMapTotalSize{312};
@@ -116,7 +115,6 @@
ASSERT_OK_AND_ASSIGN(DomStorageDatabase::Metadata metadata,
destination->ReadAllMetadata());
EXPECT_TRUE(metadata.map_metadata.empty());
- EXPECT_EQ(metadata.next_map_id, std::nullopt);
}
TEST_F(DomStorageDatabaseTest, MigrateLocalStorageWithSingleMap) {
@@ -164,7 +162,6 @@
},
};
ExpectEqualsMapMetadataSpan(metadata.map_metadata, kExpectedMapMetadata);
- EXPECT_EQ(metadata.next_map_id, std::nullopt);
}
TEST_F(DomStorageDatabaseTest, MigrateLocalStorageWithMultipleMaps) {
@@ -221,8 +218,6 @@
// Verify metadata for both maps.
ASSERT_OK_AND_ASSIGN(DomStorageDatabase::Metadata metadata,
destination->ReadAllMetadata());
-
- EXPECT_EQ(metadata.next_map_id, std::nullopt);
ASSERT_EQ(metadata.map_metadata.size(), 2u);
// Each map must have a unique ID.
@@ -298,7 +293,6 @@
};
DomStorageDatabase::Metadata metadata;
- metadata.next_map_id = kNextMapId;
metadata.map_metadata = CloneMapMetadataVector(kExpectedMapMetadata);
DbStatus status = source->PutMetadata(std::move(metadata));
@@ -328,8 +322,6 @@
// Verify metadata was migrated.
ASSERT_OK_AND_ASSIGN(DomStorageDatabase::Metadata dest_metadata,
destination->ReadAllMetadata());
-
- EXPECT_EQ(dest_metadata.next_map_id, kNextMapId);
ExpectEqualsMapMetadataSpan(dest_metadata.map_metadata, kExpectedMapMetadata);
}
@@ -345,7 +337,6 @@
expected_map_metadata[0].map_locator.AddSession(kSecondSessionId);
DomStorageDatabase::Metadata metadata;
- metadata.next_map_id = kNextMapId;
metadata.map_metadata = CloneMapMetadataVector(expected_map_metadata);
DbStatus status = source->PutMetadata(std::move(metadata));
@@ -376,8 +367,6 @@
// Verify the cloned map's metadata migrated.
ASSERT_OK_AND_ASSIGN(DomStorageDatabase::Metadata dest_metadata,
destination->ReadAllMetadata());
-
- EXPECT_EQ(dest_metadata.next_map_id, kNextMapId);
ExpectEqualsMapMetadataSpan(dest_metadata.map_metadata,
expected_map_metadata);
}
@@ -399,7 +388,6 @@
};
DomStorageDatabase::Metadata metadata;
- metadata.next_map_id = kNextMapId;
metadata.map_metadata = CloneMapMetadataVector(kExpectedMapMetadata);
DbStatus status = source->PutMetadata(std::move(metadata));
@@ -444,8 +432,6 @@
// Verify metadata for both maps.
ASSERT_OK_AND_ASSIGN(DomStorageDatabase::Metadata dest_metadata,
destination->ReadAllMetadata());
-
- EXPECT_EQ(dest_metadata.next_map_id, kNextMapId);
ExpectEqualsMapMetadataSpan(dest_metadata.map_metadata, kExpectedMapMetadata);
}
diff --git a/components/services/storage/dom_storage/leveldb/local_storage_leveldb_unittest.cc b/components/services/storage/dom_storage/leveldb/local_storage_leveldb_unittest.cc
index c6aaf2f3..878e551 100644
--- a/components/services/storage/dom_storage/leveldb/local_storage_leveldb_unittest.cc
+++ b/components/services/storage/dom_storage/leveldb/local_storage_leveldb_unittest.cc
@@ -167,7 +167,6 @@
// Read back the map usage metadata from the database.
ASSERT_OK_AND_ASSIGN(DomStorageDatabase::Metadata all_metadata,
database.ReadAllMetadata());
- EXPECT_EQ(all_metadata.next_map_id, std::nullopt);
ExpectEqualsMapMetadataSpan(all_metadata.map_metadata,
base::span_from_ref(metadata_to_update));
}
@@ -338,7 +337,6 @@
ASSERT_OK_AND_ASSIGN(DomStorageDatabase::Metadata all_metadata,
local_storage_leveldb->ReadAllMetadata());
EXPECT_EQ(all_metadata.map_metadata.size(), 0u);
- EXPECT_EQ(all_metadata.next_map_id, std::nullopt);
}
TEST_F(LocalStorageLevelDBTest, ReadAllMetadataWithInvalid) {
@@ -356,7 +354,6 @@
ASSERT_OK_AND_ASSIGN(DomStorageDatabase::Metadata all_metadata,
local_storage_leveldb->ReadAllMetadata());
EXPECT_EQ(all_metadata.map_metadata.size(), 0u);
- EXPECT_EQ(all_metadata.next_map_id, std::nullopt);
}
TEST_F(LocalStorageLevelDBTest, ReadAllMetadataWithAccessMetadata) {
@@ -382,7 +379,6 @@
},
};
ExpectEqualsMapMetadataSpan(all_metadata.map_metadata, kExpectedMapMetadata);
- EXPECT_EQ(all_metadata.next_map_id, std::nullopt);
}
TEST_F(LocalStorageLevelDBTest, ReadAllMetadataWithWriteMetadata) {
@@ -409,7 +405,6 @@
},
};
ExpectEqualsMapMetadataSpan(all_metadata.map_metadata, kExpectedMapMetadata);
- EXPECT_EQ(all_metadata.next_map_id, std::nullopt);
}
TEST_F(LocalStorageLevelDBTest, ReadAllMetadataWithWriteAndAccessMetadata) {
@@ -441,7 +436,6 @@
},
};
ExpectEqualsMapMetadataSpan(all_metadata.map_metadata, kExpectedMapMetadata);
- EXPECT_EQ(all_metadata.next_map_id, std::nullopt);
}
// Combine all previous tests into a single test using four storage
@@ -512,7 +506,6 @@
};
ExpectEqualsMapMetadataSpan(all_metadata.map_metadata,
kExpectedAllMapMetadata);
- EXPECT_EQ(all_metadata.next_map_id, std::nullopt);
}
TEST_F(LocalStorageLevelDBTest, PutMetadataWithEmpty) {
@@ -1057,8 +1050,6 @@
// Verify no metadata exists.
ASSERT_OK_AND_ASSIGN(DomStorageDatabase::Metadata all_metadata,
local_storage_leveldb->ReadAllMetadata());
-
- EXPECT_EQ(all_metadata.next_map_id, std::nullopt);
EXPECT_EQ(all_metadata.map_metadata.size(), 0u);
}
diff --git a/components/services/storage/dom_storage/leveldb/session_storage_leveldb_unittest.cc b/components/services/storage/dom_storage/leveldb/session_storage_leveldb_unittest.cc
index 36fe4ec..e1e379e 100644
--- a/components/services/storage/dom_storage/leveldb/session_storage_leveldb_unittest.cc
+++ b/components/services/storage/dom_storage/leveldb/session_storage_leveldb_unittest.cc
@@ -252,52 +252,9 @@
ASSERT_TRUE(metadata.has_value());
// The next map ID must start at zero.
- ASSERT_TRUE(metadata->next_map_id.has_value());
- EXPECT_EQ(*metadata->next_map_id, 0);
EXPECT_EQ(metadata->map_metadata.size(), 0u);
}
-TEST_F(SessionStorageLevelDBTest, ReadAllMapMetadataWithNextMapId) {
- std::unique_ptr<SessionStorageLevelDB> session_storage_leveldb;
- ASSERT_NO_FATAL_FAILURE(OpenInMemory(&session_storage_leveldb));
-
- ASSERT_NO_FATAL_FAILURE(WriteEntries(*session_storage_leveldb,
- {
- {
- ToBytes(kNextMapIdKey),
- /*value=*/ToBytes("576847"),
- },
- }));
-
- StatusOr<DomStorageDatabase::Metadata> metadata =
- session_storage_leveldb->ReadAllMetadata();
- ASSERT_TRUE(metadata.has_value());
-
- ASSERT_TRUE(metadata->next_map_id.has_value());
- EXPECT_EQ(*metadata->next_map_id, 576847);
-
- EXPECT_EQ(metadata->map_metadata.size(), 0u);
-}
-
-TEST_F(SessionStorageLevelDBTest, ReadAllMapMetadataWithNextMapIdInvalid) {
- std::unique_ptr<SessionStorageLevelDB> session_storage_leveldb;
- ASSERT_NO_FATAL_FAILURE(OpenInMemory(&session_storage_leveldb));
-
- ASSERT_NO_FATAL_FAILURE(WriteEntries(
- *session_storage_leveldb, {
- {
- ToBytes(kNextMapIdKey),
- /*value=*/ToBytes("not_a_number"),
- },
- }));
-
- StatusOr<DomStorageDatabase::Metadata> metadata =
- session_storage_leveldb->ReadAllMetadata();
-
- ASSERT_FALSE(metadata.has_value());
- EXPECT_TRUE(metadata.error().IsCorruption());
-}
-
TEST_F(SessionStorageLevelDBTest, ReadAllMapMetadata) {
std::unique_ptr<SessionStorageLevelDB> session_storage_leveldb;
ASSERT_NO_FATAL_FAILURE(OpenInMemory(&session_storage_leveldb));
@@ -315,10 +272,6 @@
session_storage_leveldb->ReadAllMetadata();
ASSERT_TRUE(metadata.has_value());
-
- ASSERT_TRUE(metadata->next_map_id.has_value());
- EXPECT_EQ(*metadata->next_map_id, 0);
-
const DomStorageDatabase::MapMetadata kExpectedMapMetadata[] = {
{
.map_locator{kFakeSessionId, kFakeUrlStorageKey, /*map_id=*/5343},
@@ -379,10 +332,6 @@
session_storage_leveldb->ReadAllMetadata();
ASSERT_TRUE(metadata.has_value());
-
- ASSERT_TRUE(metadata->next_map_id.has_value());
- EXPECT_EQ(*metadata->next_map_id, 0);
-
DomStorageDatabase::MapMetadata expected_map_metadata[] = {
{
.map_locator{kOtherFakeSessionId, kFakeUrlStorageKey,
@@ -408,13 +357,10 @@
}
TEST_F(SessionStorageLevelDBTest, PutMetadata) {
- constexpr int64_t kNextMapId = kFakeMapId + 1;
-
std::unique_ptr<SessionStorageLevelDB> session_storage_leveldb;
ASSERT_NO_FATAL_FAILURE(OpenInMemory(&session_storage_leveldb));
DomStorageDatabase::Metadata metadata;
- metadata.next_map_id = kNextMapId;
metadata.map_metadata.push_back(
{.map_locator{kFakeSessionId, kFakeUrlStorageKey, kFakeMapId}});
@@ -425,30 +371,22 @@
ASSERT_OK_AND_ASSIGN(
std::vector<DomStorageDatabase::KeyValuePair> all_entries,
session_storage_leveldb->GetLevelDBForTesting().GetPrefixed({}));
- ASSERT_EQ(all_entries.size(), 3u);
+ ASSERT_EQ(all_entries.size(), 2u);
EXPECT_EQ(all_entries[0].key,
CreateMapMetadataKey(kFakeSessionId, kFakeUrlStorageKey));
EXPECT_EQ(all_entries[0].value,
base::as_byte_span(base::NumberToString(1565)));
- EXPECT_EQ(all_entries[1].key, ToBytes(kNextMapIdKey));
- EXPECT_EQ(all_entries[1].value,
- base::as_byte_span(base::NumberToString(kNextMapId)));
-
- VerifyDatabaseVersionEntry(all_entries[2]);
+ VerifyDatabaseVersionEntry(all_entries[1]);
}
TEST_F(SessionStorageLevelDBTest, PutMetadataWithMultipleMaps) {
- constexpr int64_t kNextMapId = kOtherFakeMapId + 2;
-
std::unique_ptr<SessionStorageLevelDB> session_storage_leveldb;
ASSERT_NO_FATAL_FAILURE(OpenInMemory(&session_storage_leveldb));
// Update the metadata for 3 different maps.
DomStorageDatabase::Metadata metadata;
- metadata.next_map_id = kNextMapId;
-
... (truncated)
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