Skip to content

fix: avoid out-of-bounds array access in modelinfomgr - #230

Merged
AmeerDotEXE merged 1 commit into
user-grinch:mainfrom
CanerKaraca23:fix-modelinfomgr-bounds-check-2942288192757790142
Aug 1, 2026
Merged

fix: avoid out-of-bounds array access in modelinfomgr#230
AmeerDotEXE merged 1 commit into
user-grinch:mainfrom
CanerKaraca23:fix-modelinfomgr-bounds-check-2942288192757790142

Conversation

@CanerKaraca23

Copy link
Copy Markdown
Contributor

🎯 What: Removed a hacky data.nFrameCount <= 10 check used to workaround a crash and replaced it with strict bounds checking before accessing m_SirenStatus and m_StrobeStatus arrays based on GetSirenIndex / GetStrobeIndex.

💡 Why: The frame count check delayed the bug but didn't actually fix the root cause, which was out-of-bounds array reads. Checking the index ensures memory safety regardless of the frame counter, which is cleaner and less susceptible to race conditions.

Verification: Verified by a code reviewer confirming the logical correctness of checking array limits idx >= 0 && idx < MAX_LIGHTS before proceeding with the read. Avoided polluting the repo with compile_commands.json or plugin-sdk downloads.

Result: Improved crash resilience and robustness for vehicle material rendering without arbitrary frame delay workarounds.

Copilot AI review requested due to automatic review settings May 8, 2026 19:17

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens ModelInfoMgr::SetEditableMaterialsCB against out-of-bounds reads when resolving siren/strobe material states by validating indices returned from GetSirenIndex / GetStrobeIndex, removing the prior frame-count-based workaround.

Changes:

  • Removed the early-return “frame count <= 10” workaround for siren materials.
  • Added explicit bounds checks before reading m_SirenStatus[] and m_StrobeStatus[] using the computed indices.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/utils/modelinfomgr.cpp
Proper bounds checking for indices returned by GetSirenIndex and GetStrobeIndex before accessing the associated status arrays.
@AmeerDotEXE
AmeerDotEXE force-pushed the fix-modelinfomgr-bounds-check-2942288192757790142 branch from c5cb692 to 13faa11 Compare August 1, 2026 19:55
@AmeerDotEXE

AmeerDotEXE commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator

added back the siren check, it will be removed by the next pr. thanks for helping :D
edit: don't worry about build fail, i'll update plugin-sdk after all pr

@AmeerDotEXE
AmeerDotEXE merged commit b61a0a9 into user-grinch:main Aug 1, 2026
1 check failed
@CanerKaraca23
CanerKaraca23 deleted the fix-modelinfomgr-bounds-check-2942288192757790142 branch August 1, 2026 21:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants