From eb02425ef9678c85ff0879503859de06d68fb484 Mon Sep 17 00:00:00 2001 From: Idan Horowitz Date: Wed, 22 Jun 2022 21:06:28 +0300 Subject: [PATCH] AK: Clear the previous and next pointers of deleted HashTable buckets Usually the values of the previous and next pointers of deleted buckets are never used, as they're not part of the main ordered bucket chain, but if an in-place rehashing is done, which results in the bucket being turned into a free bucket, the stale pointers will remain, at which point any item that is inserted into said free-bucket will have either a stale previous pointer if the HashTable was empty on insertion, or a stale next pointer, resulting in undefined behaviour. This commit also includes a new HashMap test that reproduces this issue --- AK/HashTable.h | 3 ++- Tests/AK/TestHashMap.cpp | 10 ++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/AK/HashTable.h b/AK/HashTable.h index c9cfbabfa9..e961e75a0f 100644 --- a/AK/HashTable.h +++ b/AK/HashTable.h @@ -704,11 +704,12 @@ private: bucket.previous->next = bucket.next; else m_collection_data.head = bucket.next; - + bucket.previous = nullptr; if (bucket.next) bucket.next->previous = bucket.previous; else m_collection_data.tail = bucket.previous; + bucket.next = nullptr; } } diff --git a/Tests/AK/TestHashMap.cpp b/Tests/AK/TestHashMap.cpp index b7a284a8cd..8d46a3d612 100644 --- a/Tests/AK/TestHashMap.cpp +++ b/Tests/AK/TestHashMap.cpp @@ -201,3 +201,13 @@ TEST_CASE(basic_contains) EXPECT_EQ(map.remove(1), true); EXPECT_EQ(map.contains(1), false); } + +TEST_CASE(in_place_rehashing_ordered_loop_bug) +{ + OrderedHashMap map; + map.set("yt.innertube::nextId", ""); + map.set("yt.innertube::requests", ""); + map.remove("yt.innertube::nextId"); + map.set("yt.innertube::nextId", ""); + VERIFY(map.keys().size() == 2); +}