From 7ad9a64cf4c7a327c9af586750b13a78ced44f50 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Mon, 28 Sep 2026 09:07:01 -0600 Subject: [PATCH] Serialize: Clear pointer maps and sets right after deleting their values DoMap and DoSet cleared them, but only once the count had been read. A state truncated right there left the deleted pointers in place, to be freed again when the failed load reset the game. Co-Authored-By: Claude Opus 5.5 (1M context) --- Common/Serialize/SerializeMap.h | 12 ++++++++---- Common/Serialize/SerializeSet.h | 4 ++-- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/Common/Serialize/SerializeMap.h b/Common/Serialize/SerializeMap.h index 05c16a3fe5..4c1943d28f 100644 --- a/Common/Serialize/SerializeMap.h +++ b/Common/Serialize/SerializeMap.h @@ -29,8 +29,6 @@ void DoMap(PointerWrap &p, M &x, typename M::mapped_type &default_val) { switch (p.mode) { case PointerWrap::MODE_READ: { - // Clear before the guard below can bail out: for a map of pointers, our caller has - // already deleted every value, so leaving them in place would be a use-after-free. x.clear(); // Guard against an attacker-controlled count driving an enormous number of // loop iterations/allocations, same spirit as DoVector's guard. @@ -74,6 +72,8 @@ void Do(PointerWrap &p, std::map &x) { for (auto &iter : x) { delete iter.second; } + // Right away: if reading the count fails, DoMap won't get as far as clearing. + x.clear(); } T *dv = nullptr; DoMap(p, x, dv); @@ -91,6 +91,8 @@ void Do(PointerWrap &p, std::unordered_map &x) { for (auto &iter : x) { delete iter.second; } + // Right away: if reading the count fails, DoMap won't get as far as clearing. + x.clear(); } T *dv = nullptr; DoMap(p, x, dv); @@ -109,8 +111,6 @@ void DoMultimap(PointerWrap &p, M &x, typename M::mapped_type &default_val) { switch (p.mode) { case PointerWrap::MODE_READ: { - // Clear before the guard below can bail out: for a map of pointers, our caller has - // already deleted every value, so leaving them in place would be a use-after-free. x.clear(); // Guard against an attacker-controlled count driving an enormous number of // loop iterations/allocations, same spirit as DoVector's guard. @@ -153,6 +153,8 @@ void Do(PointerWrap &p, std::multimap &x) { for (auto &iter : x) { delete iter.second; } + // Right away: if reading the count fails, DoMap won't get as far as clearing. + x.clear(); } T *dv = nullptr; DoMultimap(p, x, dv); @@ -170,6 +172,8 @@ void Do(PointerWrap &p, std::unordered_multimap &x) { for (auto &iter : x) { delete iter.second; } + // Right away: if reading the count fails, DoMap won't get as far as clearing. + x.clear(); } T *dv = nullptr; DoMultimap(p, x, dv); diff --git a/Common/Serialize/SerializeSet.h b/Common/Serialize/SerializeSet.h index 018e674b29..e502a344ad 100644 --- a/Common/Serialize/SerializeSet.h +++ b/Common/Serialize/SerializeSet.h @@ -29,8 +29,6 @@ void DoSet(PointerWrap &p, std::set &x) { switch (p.mode) { case PointerWrap::MODE_READ: { - // Clear before the guard below can bail out: for a set of pointers, our caller has - // already deleted every element, so leaving them in place would be a use-after-free. x.clear(); // Guard against an attacker-controlled count driving an enormous number of // loop iterations/allocations, same spirit as DoVector's guard. @@ -65,6 +63,8 @@ void Do(PointerWrap &p, std::set &x) { for (T *s : x) { delete s; } + // Right away: if reading the count fails, DoSet won't get as far as clearing. + x.clear(); } DoSet(p, x); }