|
| 1 | +From 5a0e2cb5e3958dd90bb8569a2766622cb74d90c1 Mon Sep 17 00:00:00 2001 |
| 2 | +From: Derek Mauro <dmauro@google.com> |
| 3 | +Date: Thu, 23 Jan 2025 06:33:43 -0800 |
| 4 | +Subject: [PATCH] Fix potential integer overflow in hash container |
| 5 | + create/resize |
| 6 | + |
| 7 | +The sized constructors, reserve(), and rehash() methods of |
| 8 | +absl::{flat,node}_hash_{set,map} did not impose an upper bound on |
| 9 | +their size argument. As a result, it was possible for a caller to pass |
| 10 | +a very large size that would cause an integer overflow when computing |
| 11 | +the size of the container's backing store. Subsequent accesses to the |
| 12 | +container might then access out-of-bounds memory. |
| 13 | + |
| 14 | +The fix is in two parts: |
| 15 | + |
| 16 | +1) Update max_size() to return the maximum number of items that can be |
| 17 | +stored in the container |
| 18 | + |
| 19 | +2) Validate the size arguments to the constructors, reserve(), and |
| 20 | +rehash() methods, and abort the program when the argument is invalid |
| 21 | + |
| 22 | +We've looked at uses of these containers in Google codebases like |
| 23 | +Chrome, and determined this vulnerability is likely to be difficult to |
| 24 | +exploit. This is primarily because container sizes are rarely |
| 25 | +attacker-controlled. |
| 26 | + |
| 27 | +The bug was discovered by Dmitry Vyukov <dvyukov@google.com>. |
| 28 | + |
| 29 | +PiperOrigin-RevId: 718841870 |
| 30 | +Change-Id: Ic09dc9de140a35dbb45ab9d90f58383cf2de8286 |
| 31 | + |
| 32 | +Upstream Patch Reference: https://github.com/abseil/abseil-cpp/commit/5a0e2cb5e3958dd90bb8569a2766622cb74d90c1.patch |
| 33 | +--- |
| 34 | + .../absl/container/internal/raw_hash_set.cc | 5 +++ |
| 35 | + .../absl/container/internal/raw_hash_set.h | 36 ++++++++++++++++++- |
| 36 | + .../container/internal/raw_hash_set_test.cc | 8 +++++ |
| 37 | + 3 files changed, 48 insertions(+), 1 deletion(-) |
| 38 | + |
| 39 | +diff --git a/extra/abseil/abseil-cpp-20230802.1/absl/container/internal/raw_hash_set.cc b/extra/abseil/abseil-cpp-20230802.1/absl/container/internal/raw_hash_set.cc |
| 40 | +index 2ff95b61..58a516b6 100644 |
| 41 | +--- a/extra/abseil/abseil-cpp-20230802.1/absl/container/internal/raw_hash_set.cc |
| 42 | ++++ b/extra/abseil/abseil-cpp-20230802.1/absl/container/internal/raw_hash_set.cc |
| 43 | +@@ -23,6 +23,7 @@ |
| 44 | + #include "absl/base/config.h" |
| 45 | + #include "absl/base/dynamic_annotations.h" |
| 46 | + #include "absl/hash/hash.h" |
| 47 | ++#include "absl/base/internal/raw_logging.h" |
| 48 | + |
| 49 | + namespace absl { |
| 50 | + ABSL_NAMESPACE_BEGIN |
| 51 | +@@ -258,6 +259,10 @@ void ClearBackingArray(CommonFields& c, const PolicyFunctions& policy, |
| 52 | + } |
| 53 | + } |
| 54 | + |
| 55 | ++void HashTableSizeOverflow() { |
| 56 | ++ ABSL_RAW_LOG(FATAL, "Hash table size overflow"); |
| 57 | ++} |
| 58 | ++ |
| 59 | + } // namespace container_internal |
| 60 | + ABSL_NAMESPACE_END |
| 61 | + } // namespace absl |
| 62 | +diff --git a/extra/abseil/abseil-cpp-20230802.1/absl/container/internal/raw_hash_set.h b/extra/abseil/abseil-cpp-20230802.1/absl/container/internal/raw_hash_set.h |
| 63 | +index 5f89d8ef..ba2d98d8 100644 |
| 64 | +--- a/extra/abseil/abseil-cpp-20230802.1/absl/container/internal/raw_hash_set.h |
| 65 | ++++ b/extra/abseil/abseil-cpp-20230802.1/absl/container/internal/raw_hash_set.h |
| 66 | +@@ -236,6 +236,15 @@ namespace container_internal { |
| 67 | + #define ABSL_SWISSTABLE_ENABLE_GENERATIONS |
| 68 | + #endif |
| 69 | + |
| 70 | ++#ifdef ABSL_SWISSTABLE_ASSERT |
| 71 | ++#error ABSL_SWISSTABLE_ASSERT cannot be directly set |
| 72 | ++#else |
| 73 | ++// We use this macro for assertions that users may see when the table is in an |
| 74 | ++// invalid state that sanitizers may help diagnose. |
| 75 | ++#define ABSL_SWISSTABLE_ASSERT(CONDITION) \ |
| 76 | ++ assert((CONDITION) && "Try enabling sanitizers.") |
| 77 | ++#endif |
| 78 | ++ |
| 79 | + // We use uint8_t so we don't need to worry about padding. |
| 80 | + using GenerationType = uint8_t; |
| 81 | + |
| 82 | +@@ -939,6 +948,9 @@ inline size_t SlotOffset(size_t capacity, size_t slot_align) { |
| 83 | + // Given the capacity of a table, computes the total size of the backing |
| 84 | + // array. |
| 85 | + inline size_t AllocSize(size_t capacity, size_t slot_size, size_t slot_align) { |
| 86 | ++ ABSL_SWISSTABLE_ASSERT( |
| 87 | ++ slot_size <= |
| 88 | ++ ((std::numeric_limits<size_t>::max)() - SlotOffset(capacity, slot_align)) / capacity); |
| 89 | + return SlotOffset(capacity, slot_align) + capacity * slot_size; |
| 90 | + } |
| 91 | + |
| 92 | +@@ -1076,6 +1088,15 @@ inline size_t NormalizeCapacity(size_t n) { |
| 93 | + return n ? ~size_t{} >> countl_zero(n) : 1; |
| 94 | + } |
| 95 | + |
| 96 | ++template <size_t kSlotSize> |
| 97 | ++size_t MaxValidCapacity() { |
| 98 | ++ return NormalizeCapacity((std::numeric_limits<size_t>::max)() / 4 / |
| 99 | ++ kSlotSize); |
| 100 | ++} |
| 101 | ++ |
| 102 | ++// Use a non-inlined function to avoid code bloat. |
| 103 | ++[[noreturn]] void HashTableSizeOverflow(); |
| 104 | ++ |
| 105 | + // General notes on capacity/growth methods below: |
| 106 | + // - We use 7/8th as maximum load factor. For 16-wide groups, that gives an |
| 107 | + // average of two empty slots per group. |
| 108 | +@@ -1717,6 +1738,10 @@ class raw_hash_set { |
| 109 | + const allocator_type& alloc = allocator_type()) |
| 110 | + : settings_(CommonFields{}, hash, eq, alloc) { |
| 111 | + if (bucket_count) { |
| 112 | ++ if (ABSL_PREDICT_FALSE(bucket_count > |
| 113 | ++ MaxValidCapacity<sizeof(slot_type)>())) { |
| 114 | ++ HashTableSizeOverflow(); |
| 115 | ++ } |
| 116 | + common().set_capacity(NormalizeCapacity(bucket_count)); |
| 117 | + initialize_slots(); |
| 118 | + } |
| 119 | +@@ -1916,7 +1941,9 @@ class raw_hash_set { |
| 120 | + bool empty() const { return !size(); } |
| 121 | + size_t size() const { return common().size(); } |
| 122 | + size_t capacity() const { return common().capacity(); } |
| 123 | +- size_t max_size() const { return (std::numeric_limits<size_t>::max)(); } |
| 124 | ++ size_t max_size() const { |
| 125 | ++ return CapacityToGrowth(MaxValidCapacity<sizeof(slot_type)>()); |
| 126 | ++ } |
| 127 | + |
| 128 | + ABSL_ATTRIBUTE_REINITIALIZES void clear() { |
| 129 | + // Iterating over this container is O(bucket_count()). When bucket_count() |
| 130 | +@@ -2266,6 +2293,9 @@ class raw_hash_set { |
| 131 | + auto m = NormalizeCapacity(n | GrowthToLowerboundCapacity(size())); |
| 132 | + // n == 0 unconditionally rehashes as per the standard. |
| 133 | + if (n == 0 || m > capacity()) { |
| 134 | ++ if (ABSL_PREDICT_FALSE(m > MaxValidCapacity<sizeof(slot_type)>())) { |
| 135 | ++ HashTableSizeOverflow(); |
| 136 | ++ } |
| 137 | + resize(m); |
| 138 | + |
| 139 | + // This is after resize, to ensure that we have completed the allocation |
| 140 | +@@ -2276,6 +2306,9 @@ class raw_hash_set { |
| 141 | + |
| 142 | + void reserve(size_t n) { |
| 143 | + if (n > size() + growth_left()) { |
| 144 | ++ if (ABSL_PREDICT_FALSE(n > max_size())) { |
| 145 | ++ HashTableSizeOverflow(); |
| 146 | ++ } |
| 147 | + size_t m = GrowthToLowerboundCapacity(n); |
| 148 | + resize(NormalizeCapacity(m)); |
| 149 | + |
| 150 | +@@ -2882,5 +2915,6 @@ ABSL_NAMESPACE_END |
| 151 | + } // namespace absl |
| 152 | + |
| 153 | + #undef ABSL_SWISSTABLE_ENABLE_GENERATIONS |
| 154 | ++#undef ABSL_SWISSTABLE_ASSERT |
| 155 | + |
| 156 | + #endif // ABSL_CONTAINER_INTERNAL_RAW_HASH_SET_H_ |
| 157 | +diff --git a/extra/abseil/abseil-cpp-20230802.1/absl/container/internal/raw_hash_set_test.cc b/extra/abseil/abseil-cpp-20230802.1/absl/container/internal/raw_hash_set_test.cc |
| 158 | +index 242a97cb..d5d5f393 100644 |
| 159 | +--- a/extra/abseil/abseil-cpp-20230802.1/absl/container/internal/raw_hash_set_test.cc |
| 160 | ++++ b/extra/abseil/abseil-cpp-20230802.1/absl/container/internal/raw_hash_set_test.cc |
| 161 | +@@ -2510,6 +2510,14 @@ TEST(Iterator, InvalidComparisonDifferentTables) { |
| 162 | + "Invalid iterator comparison.*non-end"); |
| 163 | + } |
| 164 | + |
| 165 | ++TEST(Table, MaxSizeOverflow) { |
| 166 | ++ size_t overflow = (std::numeric_limits<size_t>::max)(); |
| 167 | ++ EXPECT_DEATH_IF_SUPPORTED(IntTable t(overflow), "Hash table size overflow"); |
| 168 | ++ IntTable t; |
| 169 | ++ EXPECT_DEATH_IF_SUPPORTED(t.reserve(overflow), "Hash table size overflow"); |
| 170 | ++ EXPECT_DEATH_IF_SUPPORTED(t.rehash(overflow), "Hash table size overflow"); |
| 171 | ++} |
| 172 | ++ |
| 173 | + } // namespace |
| 174 | + } // namespace container_internal |
| 175 | + ABSL_NAMESPACE_END |
| 176 | +-- |
| 177 | +2.45.4 |
| 178 | + |
0 commit comments