[Audit][Medium] ComponentStorage.remove() usize underflow when storage arrays are empty but map has stale entry

[MichaelFisher1997]

🔍 Module Scanned

(automated audit scan)

📝 Summary

The function in performs a subtraction () without checking if is zero. If called when the component/entity arrays are empty but the sparse map somehow still contains an entry (a desync condition), this causes integer underflow resulting in an out-of-bounds array access and panic.

📍 Location

• File:
• Function/Scope:

🔴 Severity: Medium

• Medium: Latent bug - while this desync cannot occur through correct API usage, defensive code should handle this case to prevent crashes if internal state becomes corrupted or through future refactorings.

💥 Impact

If the bug is triggered (via desync of map and arrays), the program will panic with an out-of-bounds array access. The desync condition cannot occur through normal API usage since , , and all keep the map and arrays synchronized. However, if a future code change introduces a bug or if internal state becomes corrupted through other means, this would cause a crash instead of a graceful error.

🔎 Evidence

If is 0, then on wraps to , causing an out-of-bounds access on the next line when is accessed.

🛠️ Proposed Fix

Add a bounds check at the start of the removal block:

Alternatively, use or add a to catch this in debug builds.

✅ Acceptance Criteria

• Add defensive bounds check in to handle empty arrays with stale map entries
• Unit tests cover edge case of remove on empty storage (can be tested by directly manipulating internal state to simulate desync)
• Code compiles and existing ECS tests pass with Zig 0.16.0 + SDL3 Dev Environment
  Compiler: 0.16.0
  assets/shaders/vulkan/sky.vert
  assets/shaders/vulkan/g_pass.frag
  assets/shaders/vulkan/ui_tex.vert
  assets/shaders/vulkan/ui.frag
  assets/shaders/vulkan/water.frag
  assets/shaders/vulkan/shadow.frag
  assets/shaders/vulkan/taa.frag
  assets/shaders/vulkan/ui.vert
  assets/shaders/vulkan/ssao_blur.frag
  assets/shaders/vulkan/lpv_inject.comp
  assets/shaders/vulkan/terrain.frag
  assets/shaders/vulkan/debug_shadow.frag
  assets/shaders/vulkan/taa.vert
  assets/shaders/vulkan/ui_tex.frag
  assets/shaders/vulkan/ssao.frag
  assets/shaders/vulkan/shadow.vert
  assets/shaders/vulkan/lpv_propagate.comp
  assets/shaders/vulkan/terrain.vert
  assets/shaders/vulkan/sky.frag
  assets/shaders/vulkan/debug_shadow.vert
  assets/shaders/vulkan/water.vert
  assets/shaders/vulkan/culling.comp
  assets/shaders/vulkan/depth_pyramid.comp
  assets/shaders/vulkan/bloom_downsample.vert
  assets/shaders/vulkan/bloom_upsample.vert
  assets/shaders/vulkan/debug_shadow.vert
  assets/shaders/vulkan/fxaa.vert
  assets/shaders/vulkan/post_process.vert
  assets/shaders/vulkan/shadow.vert
  assets/shaders/vulkan/sky.vert
  assets/shaders/vulkan/ssao.vert
  assets/shaders/vulkan/taa.vert
  assets/shaders/vulkan/terrain.vert
  assets/shaders/vulkan/ui.vert
  assets/shaders/vulkan/ui_tex.vert
  assets/shaders/vulkan/water.vert
  assets/shaders/vulkan/bloom_downsample.frag
  assets/shaders/vulkan/bloom_upsample.frag
  assets/shaders/vulkan/debug_shadow.frag
  assets/shaders/vulkan/fxaa.frag
  assets/shaders/vulkan/g_pass.frag
  assets/shaders/vulkan/post_process.frag
  assets/shaders/vulkan/shadow.frag
  assets/shaders/vulkan/sky.frag
  assets/shaders/vulkan/ssao.frag
  assets/shaders/vulkan/ssao_blur.frag
  assets/shaders/vulkan/taa.frag
  assets/shaders/vulkan/terrain.frag
  assets/shaders/vulkan/terrain_debug.frag
  assets/shaders/vulkan/ui.frag
  assets/shaders/vulkan/ui_tex.frag
  assets/shaders/vulkan/water.frag
  assets/shaders/vulkan/culling.comp
  assets/shaders/vulkan/depth_pyramid.comp
  assets/shaders/vulkan/lpv_inject.comp
  assets/shaders/vulkan/lpv_propagate.comp
  assets/shaders/vulkan/mesh.comp
  assets/shaders/vulkan/mesh.comp
  assets/shaders/vulkan/ssao.vert

📚 References

• Zig-lang reference on integer overflow: https://ziglang.org/documentation/master/#Integer-Overflow
• Related issue: Potential for similar issues in other usize arithmetic in the ECS module

[github-actions]

The audit correctly identifies a real bug in modules/engine-ecs/src/storage.zig:64-86.

Root cause analysis: In ComponentStorage.remove(), line 67 computes last_index = self.components.items.len - 1. If the arrays are empty but the sparse map still contains a stale entry (a desync condition), len is0, causing usize underflow to usize.MAX, followed by an out-of-bounds access at line 68.

Code pointer: storage.zig:67 — the subtraction self.components.items.len - 1 has no guard against zero length.

Verification: This desync cannot occur through correct API usage (set() and clear() keep map and arrays synchronized), but the report correctly notes that defensive code should handle this case to prevent crashes from internal corruption or future refactorings.

Proposed fix: Add a bounds check after line 65 to handle the empty-array stale-entry case, either returning false early or panicking with a descriptive error. A simple guard would be:

if (self.map.get(entity)) |index| {
    if (self.components.items.len == 0) return false; // or error    // ... rest of removal logic
}

Scope: This is a single-function fix with straightforward acceptance criteria. The suggested unit test covering the desync edge case is appropriate. No need to split into sub-issues.

opencode session  |  github run