[Audit][High] Dangling pointer risk: Mixer holds pointers to SoundData that can be freed by SoundManager

[MichaelFisher1997]

🔍 Module Scanned\n (automated audit scan)\n\n## 📝 Summary\nThe array stores pointers that point to structs managed by . When is called (e.g., during shutdown), it frees all sound buffers AND the that stores the structs. However, the continues to hold dangling pointers to these deallocated structs, creating a use-after-free vulnerability. The null check does NOT protect against this because the pointer is not null - it points to freed memory.\n\n## 📍 Location\n- File: (Voice struct definition)\n- File: (mix() dereference of potentially freed pointer)\n- File: (deinit that frees SoundData backing store)\n- File: (deinit order issue)\n\n## 🔴 Severity: High\n- High: Memory leaks, race conditions, incorrect rendering, broken features\n\n## 💥 Impact\nWhen is called:\n1. is called first (line 65)\n2. frees all buffers and the ArrayList backing store\n3. is called afterwards (line 68)\n4. But calls which dereferences - now pointing to freed memory\n\nThis causes a use-after-free that can lead to:\n- Crashes (segmentation fault)\n- Memory corruption\n- Undefined behavior in audio processing (random audio artifacts)\n- Potential security vulnerabilities\n\nThe issue is also present during normal operation if sounds are ever unloaded while voices are still active.\n\n## 🔎 Evidence\nVoice struct holds nullable pointer to externally-managed SoundData:\n\n\nMixer's mix() dereferences sound_data pointer without validity check:\n\n\nSoundManager.deinit() frees the backing store:\n\n\nAudioSystem.deinit() calls manager.deinit() before backend.destroy():\n\n\n## 🛠️ Proposed Fix\n\n1. Fix deinit order in AudioSystem.deinit() - Destroy backend (and thus Mixer) BEFORE calling :\n\n\n2. Add null-safety check in Mixer.mix() - The current check only catches explicit , not dangling pointers:\n\n\n3. Consider clearing sound_data pointers in stopVoice() and when deactivating voices to prevent stale references.\n\n## ✅ Acceptance Criteria\n- [ ] destroys backend (Mixer) before freeing SoundManager sounds\n- [ ] No use-after-free when calling with active voices\n- [ ] The fix has been verified with Zig 0.16.0 + SDL3 Dev Environment

Compiler: 0.16.0
assets/shaders/vulkan/lpv_propagate.comp
assets/shaders/vulkan/terrain.frag
assets/shaders/vulkan/debug_shadow.frag
assets/shaders/vulkan/ssao.frag
assets/shaders/vulkan/sky.vert
assets/shaders/vulkan/debug_shadow.vert
assets/shaders/vulkan/culling.comp
assets/shaders/vulkan/terrain.vert
assets/shaders/vulkan/g_pass.frag
assets/shaders/vulkan/ui_tex.frag
assets/shaders/vulkan/taa.vert
assets/shaders/vulkan/lpv_inject.comp
assets/shaders/vulkan/mesh.comp
assets/shaders/vulkan/water.frag
assets/shaders/vulkan/ssao_blur.frag
assets/shaders/vulkan/ui_tex.vert
assets/shaders/vulkan/ui.vert
assets/shaders/vulkan/ui.frag
assets/shaders/vulkan/ssao.vert
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/shadow.vert
assets/shaders/vulkan/water.vert
assets/shaders/vulkan/taa.frag
assets/shaders/vulkan/sky.frag
assets/shaders/vulkan/shadow.frag
assets/shaders/vulkan/depth_pyramid.comp\n- [ ] No memory safety issues detected when running with valgrind or similar tooling\n\n## 📚 References\n- Zig Documentation on - Cleanup pattern used in this codebase\n- Use-after-free CWE-416 - Related security vulnerability class\n- SDL_AudioStream documentation regarding callback safety considerations\n

[github-actions]

This is a valid Bug (use-after-free vulnerability) in the audio system.

Analysis:

The dangling pointer flow is confirmed:

1. Voice.sound_data (sdl_audio.zig:26) stores raw pointers to SoundData managed externally by SoundManager

2. AudioSystem.deinit() (system.zig:63-71) shutdown order:

   • Line 64: self.stopAll() → mixer.stopAll() only sets voice.active = false — does NOT clear sound_data pointers
   • Line 65: self.manager.deinit() → frees all SoundData buffers and ArrayList backing store
   • Line 66-68: Backend destroyed (Mixer freed)

3. Mixer.mix() (sdl_audio.zig:241) null check:

   if (voice.sound_data == null) { voice.active = false; continue; }

   This only catches explicitly null pointers — a dangling pointer (pointing to freed memory) is not null, so the check passes and dereferencing voice.sound_data.? at line 246 causes use-after-free.

Root cause: mixer.stopAll() deactivates voices but doesn't null sound_data, so voices retain pointers to freed SoundData after manager.deinit().

Suggested fix (do not implement per instructions):

1. Primary: In AudioSystem.deinit(), destroy backend/Mixer before manager.deinit()
2. Secondary: Mixer.stopAll() should also clear sound_data = null to prevent stale references
3. Defense-in-depth: Consider adding a generation/validity check or sentinel in Mixer.mix()

opencode session  |  github run