[Bugfix] entity.at() returns GridPointState with dangling raw pointer into gridstate vector #264

New issue

Closed

opened 2026-03-07 23:19:53 +00:00 by john · 0 comments

john commented

2026-03-07 23:19:53 +00:00

Owner

Summary

UIEntity::at(x, y) returns a GridPointState Python object that holds a raw UIGridPointState* pointer directly into the entity's gridstate vector. If the entity changes grids (triggering gridstate.resize()), the vector may reallocate its internal storage, leaving the GridPointState object with a dangling pointer. Any subsequent access through it is undefined behavior.

Root Cause

UIEntity.cpp:130:

auto obj = (PyUIGridPointStateObject*)type->tp_alloc(type, 0);
obj->data = &(self->data->gridstate[y * self->data->grid->grid_w + x]);

The PyUIGridPointStateObject struct stores:

UIGridPointState* data;  // Raw pointer into entity's gridstate vector
UIGrid* grid;            // Also a raw pointer

When gridstate is resized (via set_grid(), EntityCollection.append(), etc.), std::vector may reallocate, invalidating all pointers into the old storage.

Reproduction

import mcrfpy

grid_a = mcrfpy.Grid(grid_size=(10, 10))
grid_b = mcrfpy.Grid(grid_size=(50, 50))

entity = mcrfpy.Entity((5, 5), grid=grid_a)
entity.update_visibility()

state = entity.at(3, 3)  # Raw pointer into gridstate[33]
print(state.visible)      # OK - vector still at original address

entity.grid = grid_b      # gridstate resizes from 100 to 2500 entries
                           # vector likely reallocates

print(state.visible)       # DANGLING POINTER - reads freed memory
state.visible = True       # DANGLING POINTER - writes to freed memory

Fix Options

Store index instead of pointer: Change GridPointState to store (entity_weak_ptr, x, y) and compute the pointer on each access
Store shared reference: Give GridPointState a shared_ptr<UIEntity> and recompute the pointer on access
Invalidation sentinel: Add a generation counter to gridstate; check it on access

Option 1 is simplest and safest.

Severity

Critical — use-after-free / dangling pointer leading to undefined behavior. Can corrupt memory silently or crash.

## Summary `UIEntity::at(x, y)` returns a `GridPointState` Python object that holds a raw `UIGridPointState*` pointer directly into the entity's `gridstate` vector. If the entity changes grids (triggering `gridstate.resize()`), the vector may reallocate its internal storage, leaving the `GridPointState` object with a dangling pointer. Any subsequent access through it is undefined behavior. ## Root Cause `UIEntity.cpp:130`: ```cpp auto obj = (PyUIGridPointStateObject*)type->tp_alloc(type, 0); obj->data = &(self->data->gridstate[y * self->data->grid->grid_w + x]); ``` The `PyUIGridPointStateObject` struct stores: ```cpp UIGridPointState* data; // Raw pointer into entity's gridstate vector UIGrid* grid; // Also a raw pointer ``` When `gridstate` is resized (via `set_grid()`, `EntityCollection.append()`, etc.), `std::vector` may reallocate, invalidating all pointers into the old storage. ## Reproduction ```python import mcrfpy grid_a = mcrfpy.Grid(grid_size=(10, 10)) grid_b = mcrfpy.Grid(grid_size=(50, 50)) entity = mcrfpy.Entity((5, 5), grid=grid_a) entity.update_visibility() state = entity.at(3, 3) # Raw pointer into gridstate[33] print(state.visible) # OK - vector still at original address entity.grid = grid_b # gridstate resizes from 100 to 2500 entries # vector likely reallocates print(state.visible) # DANGLING POINTER - reads freed memory state.visible = True # DANGLING POINTER - writes to freed memory ``` ## Fix Options 1. **Store index instead of pointer**: Change `GridPointState` to store `(entity_weak_ptr, x, y)` and compute the pointer on each access 2. **Store shared reference**: Give `GridPointState` a `shared_ptr<UIEntity>` and recompute the pointer on access 3. **Invalidation sentinel**: Add a generation counter to `gridstate`; check it on access Option 1 is simplest and safest. ## Severity **Critical** — use-after-free / dangling pointer leading to undefined behavior. Can corrupt memory silently or crash.