Bug #2161
closedHeap buffer overflow in worklist_load() via unbounded wl_length from save file -- overwrites struct city fields including pointers (CVSS 8.8)
0%
Description
Reported by Tristan.
========================================================================
Heap Buffer Overflow in worklist_load() via Crafted Save File
========================================================================
CVSS 3.1: 8.8 (AV:L/AC:L/PR:N/UI:R/S:C/C:H/I:H/A:H)
CWE: CWE-122 (Heap-based Buffer Overflow)
Auth: None (open save file)
Version: commit 815c1f4 (master, 2026-07-24)
File: server/savegame/savegame3.c, worklist_load(), lines 966-1006
server/savegame/savegame2.c, worklist_load(), lines 790-830
Root Cause:
worklist_load() reads wl_length from the save file and uses it as
a loop bound without validating against MAX_LEN_WORKLIST (64). The
loop writes struct universal entries directly to pwl->entries[i],
overflowing when i >= 64.
pwl->length = secfile_lookup_int_default(file, 0, // line 982
"%s.wl_length", path_str);
// NO check: pwl->length <= MAX_LEN_WORKLISTfor (i = 0; i < pwl->length; i++) { // line 985
kind = secfile_lookup_str(file, "%s.wl_kind%d", path_str, i);
name = secfile_lookup_str_default(file, "-", "%s.wl_value%d",
path_str, i);
pwl->entries[i] = universal_by_rule_name(kind, name); // line
992: OOB write
}The wlist_max_length parameter is only used for a padding loop
(line 1002), NOT to cap pwl->length.
Heap Layout (struct city, common/city.h:399+):
struct worklist worklist; // entries64 -- THE OVERFLOW SOURCE
bv_city_options city_options; // overwritten first
enum city_wl_cancel_behavior wlcb;
struct unit_list *units_supported; // POINTER -- corruptible for RCE
int *counter_values; // POINTER -- corruptible
When wl_length > 64, struct universal values (enum kind + union
value with pointers) overwrite city_options, wlcb, units_supported,
and counter_values. Corrupting units_supported leads to controlled
dereference when the server iterates supported units.
Contrast with safe pattern:
worklist_append() in common/worklist.c:147 correctly checks
next_index >= MAX_LEN_WORKLIST. But worklist_load() bypasses
worklist_append() entirely.
Impact:
Heap corruption via crafted save file. A save file with
wl_length=200 overwrites 136 entries past the buffer, corrupting
pointers. Arbitrary code execution via controlled pointer
dereference. Affects single-player and multiplayer (shared saves).
Both savegame v2 and v3 formats affected. All platforms.
Files