Title: [292836] trunk/Source/WebCore
- Revision
- 292836
- Author
- sihui_...@apple.com
- Date
- 2022-04-13 15:44:04 -0700 (Wed, 13 Apr 2022)
Log Message
StorageMap::importItems may update currentSize wrongly in release build
https://bugs.webkit.org/show_bug.cgi?id=239255
Reviewed by Chris Dumez.
StorageMap::importItems() has a debug assertion that keys of imported items do not exist in map, but that does
not prevent this from happening in release build. With our current implementation, if this happens in release
build, we increase currentSize without updating the map, which may lead to overflow.
To make StorageMap more safe to use, we can calculate the correct currentSize based on size of exsiting item, as
in StorageMap::setItem(), or we just don't update currentSize or map if key already exists. Since current users
of StorageMap should only call importItems() when the map is empty, we can add an early return if map is not
empty.
* storage/StorageMap.cpp:
(WebCore::StorageMap::importItems):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (292835 => 292836)
--- trunk/Source/WebCore/ChangeLog 2022-04-13 22:43:34 UTC (rev 292835)
+++ trunk/Source/WebCore/ChangeLog 2022-04-13 22:44:04 UTC (rev 292836)
@@ -1,3 +1,22 @@
+2022-04-13 Sihui Liu <sihui_...@apple.com>
+
+ StorageMap::importItems may update currentSize wrongly in release build
+ https://bugs.webkit.org/show_bug.cgi?id=239255
+
+ Reviewed by Chris Dumez.
+
+ StorageMap::importItems() has a debug assertion that keys of imported items do not exist in map, but that does
+ not prevent this from happening in release build. With our current implementation, if this happens in release
+ build, we increase currentSize without updating the map, which may lead to overflow.
+
+ To make StorageMap more safe to use, we can calculate the correct currentSize based on size of exsiting item, as
+ in StorageMap::setItem(), or we just don't update currentSize or map if key already exists. Since current users
+ of StorageMap should only call importItems() when the map is empty, we can add an early return if map is not
+ empty.
+
+ * storage/StorageMap.cpp:
+ (WebCore::StorageMap::importItems):
+
2022-04-13 Commit Queue <commit-qu...@webkit.org>
Unreviewed, reverting r292817.
Modified: trunk/Source/WebCore/storage/StorageMap.cpp (292835 => 292836)
--- trunk/Source/WebCore/storage/StorageMap.cpp 2022-04-13 22:43:34 UTC (rev 292835)
+++ trunk/Source/WebCore/storage/StorageMap.cpp 2022-04-13 22:44:04 UTC (rev 292836)
@@ -161,23 +161,16 @@
void StorageMap::importItems(HashMap<String, String>&& items)
{
- CheckedUint32 newSize = m_impl->currentSize;
- if (m_impl->map.isEmpty()) {
- m_impl->map = WTFMove(items);
- for (auto& [key, value] : m_impl->map) {
- newSize += key.sizeInBytes();
- newSize += value.sizeInBytes();
- }
- m_impl->currentSize = newSize;
- return;
- }
+ RELEASE_ASSERT(m_impl->map.isEmpty());
+ RELEASE_ASSERT(!m_impl->currentSize);
+ CheckedUint32 newSize;
for (auto& [key, value] : items) {
newSize += key.sizeInBytes();
newSize += value.sizeInBytes();
- auto result = m_impl->map.add(WTFMove(key), WTFMove(value));
- ASSERT_UNUSED(result, result.isNewEntry);
}
+
+ m_impl->map = WTFMove(items);
m_impl->currentSize = newSize;
}
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
