[Bugfix] Potential null pointer dereference in HeightMap layer operations #214

New issue

Open

opened 2026-01-12 03:49:01 +00:00 by john · 0 comments

john commented

2026-01-12 03:49:01 +00:00

Owner

Problem

The HeightMap-to-layer application methods (apply_threshold, apply_gradient, apply_ranges) do not verify that the HeightMap's internal TCOD pointer is non-null before dereferencing it.

Location 1: ValidateHeightMapSize (GridLayers.cpp:50-61)

static bool ValidateHeightMapSize(PyHeightMapObject* hmap, int grid_x, int grid_y) {
    int hmap_width = hmap->heightmap->w;  // CRASH if heightmap is nullptr
    int hmap_height = hmap->heightmap->h;
    // ...
}

Location 2: Apply loops (GridLayers.cpp, multiple locations)

float value = TCOD_heightmap_get_value(hmap->heightmap, x, y);  // CRASH if nullptr

When Can This Happen?

A PyHeightMapObject can have heightmap == nullptr if:

Memory allocation failed during TCOD_heightmap_new() in init
The object was created but init raised an exception
(Theoretically) Object corruption

While rare in practice, this violates defensive programming principles and could cause hard-to-diagnose segfaults.

Reproduction (Theoretical)

# If memory allocation fails during HeightMap creation:
import mcrfpy
hmap = mcrfpy.HeightMap((1000000, 1000000))  # Might fail allocation
layer.apply_threshold(hmap, (0, 1), 5)  # Would segfault

Proposed Fix

Add null check before ValidateHeightMapSize in each apply method:

PyHeightMapObject* hmap;
if (!IsHeightMapObject(source_obj, &hmap)) {
    PyErr_SetString(PyExc_TypeError, "source must be a HeightMap");
    return NULL;
}

// ADD THIS CHECK:
if (!hmap->heightmap) {
    PyErr_SetString(PyExc_RuntimeError, "HeightMap is not initialized");
    return NULL;
}

if (!ValidateHeightMapSize(hmap, self->data->grid_x, self->data->grid_y)) {
    return NULL;
}

Affected Methods

TileLayer.apply_threshold() (GridLayers.cpp:1781)
TileLayer.apply_ranges() (GridLayers.cpp:1834)
ColorLayer.apply_threshold() (GridLayers.cpp:1233)
ColorLayer.apply_gradient() (GridLayers.cpp:1292)
ColorLayer.apply_ranges() (GridLayers.cpp:1358)

Discovered during code review of #200 and #201 implementations.

## Problem The HeightMap-to-layer application methods (`apply_threshold`, `apply_gradient`, `apply_ranges`) do not verify that the HeightMap's internal TCOD pointer is non-null before dereferencing it. ### Location 1: ValidateHeightMapSize (GridLayers.cpp:50-61) ```cpp static bool ValidateHeightMapSize(PyHeightMapObject* hmap, int grid_x, int grid_y) { int hmap_width = hmap->heightmap->w; // CRASH if heightmap is nullptr int hmap_height = hmap->heightmap->h; // ... } ``` ### Location 2: Apply loops (GridLayers.cpp, multiple locations) ```cpp float value = TCOD_heightmap_get_value(hmap->heightmap, x, y); // CRASH if nullptr ``` ## When Can This Happen? A `PyHeightMapObject` can have `heightmap == nullptr` if: 1. Memory allocation failed during `TCOD_heightmap_new()` in init 2. The object was created but init raised an exception 3. (Theoretically) Object corruption While rare in practice, this violates defensive programming principles and could cause hard-to-diagnose segfaults. ## Reproduction (Theoretical) ```python # If memory allocation fails during HeightMap creation: import mcrfpy hmap = mcrfpy.HeightMap((1000000, 1000000)) # Might fail allocation layer.apply_threshold(hmap, (0, 1), 5) # Would segfault ``` ## Proposed Fix Add null check before `ValidateHeightMapSize` in each apply method: ```cpp PyHeightMapObject* hmap; if (!IsHeightMapObject(source_obj, &hmap)) { PyErr_SetString(PyExc_TypeError, "source must be a HeightMap"); return NULL; } // ADD THIS CHECK: if (!hmap->heightmap) { PyErr_SetString(PyExc_RuntimeError, "HeightMap is not initialized"); return NULL; } if (!ValidateHeightMapSize(hmap, self->data->grid_x, self->data->grid_y)) { return NULL; } ``` ## Affected Methods - `TileLayer.apply_threshold()` (GridLayers.cpp:1781) - `TileLayer.apply_ranges()` (GridLayers.cpp:1834) - `ColorLayer.apply_threshold()` (GridLayers.cpp:1233) - `ColorLayer.apply_gradient()` (GridLayers.cpp:1292) - `ColorLayer.apply_ranges()` (GridLayers.cpp:1358) ## Related Discovered during code review of #200 and #201 implementations.

john added the

Tiny Feature

label

2026-01-12 03:49:17 +00:00

No milestone

No project

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#214

No description provided.

Rows
Columns