Bug #2162
closedOut-of-bounds read in activities.order[] indexing -- missing bounds check that all other .order[] accesses have (CVSS 7.1)
0%
Description
Reported by Tristan
========================================================================
Out-of-Bounds Read in activities.order[] via Crafted Save File
========================================================================
CVSS 3.1: 7.1 (AV:L/AC:L/PR:N/UI:R/S:U/C:H/I:N/A:H)
CWE: CWE-125 (Out-of-bounds Read)
Auth: None (open save file)
Version: commit 815c1f4 (master, 2026-07-24)
File: server/savegame/savegame3.c, sg_load_player_unit(), line 6223
server/savegame/savegame2.c, sg_load_player_unit(), line 4290
Root Cause:
The ei integer read from a save file is used to index into
loading->activities.order[ei] without any bounds check. Every
other .order[] access in the same function validates
idx >= 0 && idx < loading->*.size before dereferencing.
sg_warn_ret_val(secfile_lookup_int(loading->file, &ei,
"%s.activity", unitstr), FALSE, ...);
activity = unit_activity_by_name(loading->activities.order[ei], // line 6223
fc_strcasecmp); //
NO bounds check
Compare with the action access immediately below (line 6230):
} else if (ei >= 0 && ei < loading->action.size) { // BOUNDS CHECKED
action = loading->action.order[ei];
All other .order[] accesses follow the same pattern:
- action.order: checked (line 6230)
- extra.order: checked (line 6248)
- ssa.order: checked (line 6365)
- act_dec.order: checked (line 6410)
- activities.order: NOT CHECKED (line 6223) -- the only one
Data Flow:
Save file with unit entry: activity=999999
> secfile_lookup_int() reads ei=999999>activities.order999999 reads past heap array
-> loading
-> Result (char*) passed to unit_activity_by_name()
-> fc_strcasecmp() dereferences the OOB pointer as a string
Impact:
- Crash (segfault on unmapped memory)
- Heap content leak via OOB pointer used as string
- Both savegame v2 and v3. All platforms.
Files