Stop MAPSTRINGS from reading past the property lists - #368
Merged
Merged
Conversation
The unknown-property error path prints every valid name by walking the list until it finds a NULL entry. None of the property lists end with one, so the loop runs off the array and printf crashes in strlen. Use MAXINDEX, the list length the macro already receives for searchList, as the bound. Scripts that name a property from another engine build now get the intended log message instead of a segfault. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request
General Description
Fixes a crash in the script error path. When a script names a property that a
MAPSTRINGSlist does not contain, the macro prints the list of valid names withNone of the 32 property lists in
openborscript.candsource/openborscript/*.cend with aNULLentry, so the loop walks past the array andprintf("%s")dereferences whatever follows. On Android (arm64, build from current master) this segfaults instrleninsidevfprintf, so the player gets a crash instead of the intended "Property name 'x' is not supported" message and list.Seen with two mods that target other engine builds: one calls
openborvariant("cheats"), the otherchangeentityproperty(self, "nextforcedirection", ...). Both now log the error and shut down cleanly as designed.The macro already receives the list length as
MAXINDEX(the same value passed tosearchList), so the loop stops there instead. One line, no behaviour change on the success path.🤖 Generated with Claude Code