john/McRogueFace
1
0
Fork 0
Code Issues 26 Pull requests Projects Releases Wiki Activity

[Meta] Engine memory safety audit — 7DRL 2026 post-mortem #279

New issue
Closed
opened 2026-03-07 23:26:21 +00:00 by john · 1 comment
john commented 2026-03-07 23:26:21 +00:00
Owner
Copy link

Summary

During 7DRL 2026 (Liber Noster), a heap buffer overflow was discovered when entities moved between grids of different sizes. The root cause was UIEntity::set_grid() only resizing gridstate when it was empty (size() == 0), leading to out-of-bounds writes in updateVisibility().

A comprehensive audit of the engine revealed 20 bugs in the same family — buffer overflows, dangling pointers, reference leaks, and thread safety issues. This meta-issue tracks the full set.

Bug Inventory

Critical — Heap Buffer Overflow (gridstate resize)

All have the same root cause: gridstate not resized when entity moves to a different-sized grid.

Issue Entry Point Location
#258 EntityCollection.append() UIEntityCollection.cpp:582
#259 EntityCollection.extend() UIEntityCollection.cpp:687
#260 EntityCollection.insert() UIEntityCollection.cpp:807
#261 EntityCollection.__setitem__ UIEntityCollection.cpp:191
#262 Contiguous slice assignment UIEntityCollection.cpp:494
#263 Extended slice assignment UIEntityCollection.cpp:519

Critical — Dangling Pointers

Issue Object Location
#264 entity.at() → GridPointState UIEntity.cpp:130
#265 grid.at() → GridPoint UIGrid.cpp:1362,1403

High — Memory Leaks & NULL Derefs

Issue Bug Location
#266 UIEntity self reference cycle UIEntity.cpp:248
#267 PyObject_GetAttrString leaks (60+ sites) Throughout
#268 sfVector2f_to_PyObject NULL deref UIEntity.cpp:298
#274 set_grid() missing spatial hash ops UIEntity.cpp:691-717
#275 UIEntity missing tp_dealloc UIEntity.h:132-172

Medium — Races, Dangling Pointers, Iterator Safety

Issue Bug Location
#269 PythonObjectCache::lookup() no mutex PythonObjectCache.cpp:37
#270 GridLayer::parent_grid dangling GridLayers.h:34
#271 UIGridPoint::parent_grid dangling UIGridPoint.h:44
#272 UniformCollection unchecked weak_ptr PyUniformCollection.h:86
#273 entity.die() during iteration UIEntity.cpp:761-788
#276 updateVisibility() defense-in-depth UIEntity.cpp:41
#277 GridChunk::parent_grid dangling GridChunk.h:39
#278 entity.at() defense-in-depth UIEntity.cpp:112

Fix Strategy

  1. Immediate (blocks next release): Fix #258-#263 (gridstate resize in all entry points) — these are active crash bugs
  2. Soon: Fix #266, #275 (entity memory leak) and #274 (spatial hash) — correctness issues
  3. Next pass: Fix #267, #268 (reference leaks and NULL checks) — hardening
  4. Ongoing: Fix #264, #265, #270-#272, #277 (dangling pointer patterns) — architectural improvement

Consolidation Opportunity

The gridstate resize bugs (#258-#263, #276, #278) could be fixed with a single helper function:

void UIEntity::ensureGridstateSize() {
    if (!grid) return;
    size_t expected = grid->grid_w * grid->grid_h;
    if (gridstate.size() != expected) {
        gridstate.resize(expected);
        for (auto& s : gridstate) { s.visible = false; s.discovered = false; }
    }
}

Called from every grid-transfer entry point and at the top of updateVisibility() and at().

## Summary During 7DRL 2026 (Liber Noster), a heap buffer overflow was discovered when entities moved between grids of different sizes. The root cause was `UIEntity::set_grid()` only resizing `gridstate` when it was empty (`size() == 0`), leading to out-of-bounds writes in `updateVisibility()`. A comprehensive audit of the engine revealed **20 bugs** in the same family — buffer overflows, dangling pointers, reference leaks, and thread safety issues. This meta-issue tracks the full set. ## Bug Inventory ### Critical — Heap Buffer Overflow (gridstate resize) All have the same root cause: gridstate not resized when entity moves to a different-sized grid. | Issue | Entry Point | Location | |-------|-------------|----------| | #258 | `EntityCollection.append()` | UIEntityCollection.cpp:582 | | #259 | `EntityCollection.extend()` | UIEntityCollection.cpp:687 | | #260 | `EntityCollection.insert()` | UIEntityCollection.cpp:807 | | #261 | `EntityCollection.__setitem__` | UIEntityCollection.cpp:191 | | #262 | Contiguous slice assignment | UIEntityCollection.cpp:494 | | #263 | Extended slice assignment | UIEntityCollection.cpp:519 | ### Critical — Dangling Pointers | Issue | Object | Location | |-------|--------|----------| | #264 | `entity.at()` → GridPointState | UIEntity.cpp:130 | | #265 | `grid.at()` → GridPoint | UIGrid.cpp:1362,1403 | ### High — Memory Leaks & NULL Derefs | Issue | Bug | Location | |-------|-----|----------| | #266 | UIEntity self reference cycle | UIEntity.cpp:248 | | #267 | PyObject_GetAttrString leaks (60+ sites) | Throughout | | #268 | sfVector2f_to_PyObject NULL deref | UIEntity.cpp:298 | | #274 | set_grid() missing spatial hash ops | UIEntity.cpp:691-717 | | #275 | UIEntity missing tp_dealloc | UIEntity.h:132-172 | ### Medium — Races, Dangling Pointers, Iterator Safety | Issue | Bug | Location | |-------|-----|----------| | #269 | PythonObjectCache::lookup() no mutex | PythonObjectCache.cpp:37 | | #270 | GridLayer::parent_grid dangling | GridLayers.h:34 | | #271 | UIGridPoint::parent_grid dangling | UIGridPoint.h:44 | | #272 | UniformCollection unchecked weak_ptr | PyUniformCollection.h:86 | | #273 | entity.die() during iteration | UIEntity.cpp:761-788 | | #276 | updateVisibility() defense-in-depth | UIEntity.cpp:41 | | #277 | GridChunk::parent_grid dangling | GridChunk.h:39 | | #278 | entity.at() defense-in-depth | UIEntity.cpp:112 | ## Fix Strategy 1. **Immediate** (blocks next release): Fix #258-#263 (gridstate resize in all entry points) — these are active crash bugs 2. **Soon**: Fix #266, #275 (entity memory leak) and #274 (spatial hash) — correctness issues 3. **Next pass**: Fix #267, #268 (reference leaks and NULL checks) — hardening 4. **Ongoing**: Fix #264, #265, #270-#272, #277 (dangling pointer patterns) — architectural improvement ## Consolidation Opportunity The gridstate resize bugs (#258-#263, #276, #278) could be fixed with a single helper function: ```cpp void UIEntity::ensureGridstateSize() { if (!grid) return; size_t expected = grid->grid_w * grid->grid_h; if (gridstate.size() != expected) { gridstate.resize(expected); for (auto& s : gridstate) { s.visible = false; s.discovered = false; } } } ``` Called from every grid-transfer entry point and at the top of `updateVisibility()` and `at()`.
john commented 2026-04-10 05:35:16 +00:00
Author
Owner
Copy link

All 20 bugs (#258–#278) are now fixed. The final three dangling pointer issues (#270, #271, #277) were resolved in commit 2f4928c by nulling parent_grid pointers in GridData::~GridData(). Closing this meta issue.

All 20 bugs (#258–#278) are now fixed. The final three dangling pointer issues (#270, #271, #277) were resolved in commit 2f4928c by nulling parent_grid pointers in GridData::~GridData(). Closing this meta issue.
john closed this issue 2026-04-10 05:35:20 +00:00
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
master
threedee
emscripten-mcrogueface
rogueliketutorial25
alpha_presentable
alpha_streamline_2
alpha_streamline_1
interpreter_mode
grid_entity_integration
reprs_and_member_names
break_up_ui_h
standardize_font_handling
standardize_vector_handling
standardize_color_handling
standardize_texture_handling
raii_pyobjects
rebase-for-7DRL_Feb-2024
engjam_uicollection
engjam_ui_overhaul
0.2.3-prerelease-7drl2026-emscripten
Labels
Clear labels   
Alpha Release Requirement

This issue is a blocker to 0.1 Alpha release of McRogueFace.

  
Bugfix

   
Demo Target

Functionality to demonstrate; Depends on new features, but this issue isn't for writing them.

  
Documentation

   
Major Feature

Significant time and effort required.

  
Minor Feature

Some effort required to create or overhaul functionality.

  
priority:tier1-active

Current development focus - critical path to v1.0

  
priority:tier2-foundation

Important foundation work - not blocking v1.0

  
priority:tier3-future

Future features - deferred until after v1.0

  
priority:tier4-deferred

   
Refactoring & Cleanup

no new functionality, just improving the codebase.

  
system:animation

Animation and property interpolation

  
system:documentation

Documentation infrastructure

  
system:grid

Grid system and spatial containers

  
system:input

Input handling and events

  
system:performance

Performance optimization and profiling

  
system:procgen

Procedural Generation (Noise, BSP, Heightmap)

  
system:python-binding

Python/C++ binding layer

  
system:rendering

Rendering pipeline and visuals

  
system:ui-hierarchy

UI component hierarchy and composition

  
Tiny Feature

quick and easy - a few lines or with little interconnection around the codebase

  
workflow:blocked

Blocked by other work - waiting on dependencies

  
workflow:needs-benchmark

Needs performance testing and benchmarks

  
workflow:needs-documentation

Needs documentation before or after implementation

No labels
Alpha Release Requirement
Bugfix
Demo Target
Documentation
Major Feature
Minor Feature
priority:tier1-active
priority:tier2-foundation
priority:tier3-future
priority:tier4-deferred
Refactoring & Cleanup
system:animation
system:documentation
system:grid
system:input
system:performance
system:procgen
system:python-binding
system:rendering
system:ui-hierarchy
Tiny Feature
workflow:blocked
workflow:needs-benchmark
workflow:needs-documentation
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
john/McRogueFace#279
No description provided.