Repository navigation
Harden model file loading against crashes - #7905
Open
dustinspace217 wants to merge 5 commits into
Open
dustinspace217 wants to merge 5 commits into
dustinspace217 wants to merge 5 commits into
Conversation
Model property values, vector properties, keyword lookups and the glow nebula texture name are now read within the property text and the fields they fill. Counts read from a model file are limited to what the file can hold, and live debris, bay paths, detail level names and shield meshes are checked before use. Malformed values now produce a warning; valid models load as before.
dustinspace217
changed the title
A glow point bank whose parent submodel number is past the end of the submodel list now warns and stops following a submodel. A $look_at or $parent_submodel given as a number is now parsed with strtol and must name an existing submodel (or be -1); anything else gets the same "unable to match" warning as an unknown name. Valid models load as before. Adds unit tests for resolve_submodel_index.
…rces A chunk with a negative length now stops the chunk loop with a warning instead of seeking back to the same chunk. Shield collision trees are checked once at load (node sizes, child offsets, polygon numbers against the shield mesh; old SLDC trees also before conversion), and a malformed tree is dropped with a warning, so collisions fall back to checking every shield triangle. Their sizes are limited to the bytes left in the file. Virtual POF glowpoint and engine sources given by number must exist, as weapon bank sources already must. Valid models load as before.
The load-time tree check now also rejects trees deeper than 256 levels (mc_check_sldc recurses once per level; installed trees are at most 16 deep) and trees that reach the same node twice, and accepts an empty 36-byte leaf. A tree whose bytes come back short from the file is dropped. A chunk length that would overflow the next chunk position stops the chunk loop like a negative one, and a virtual POF engine lookup by subsystem name checks for an empty list first. Note: a model with a zero-size tree used to ignore shield hits entirely; it now falls back to checking every shield triangle. Adds unit tests for both tree checks.
The load-time tree check now marks every byte a node covers and rejects overlapping nodes, so validation and every collision test stay linear in the tree size. convert_sldc_to_slc2 finds back targets with a search over node starts instead of walking node by node (same result: the first node start at or after the target), and an SLDC back offset must name a node before the end of the tree. The chunk loop computes the next chunk position only after checking the length. More unit tests for both tree formats.
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
1 participant
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.
What?
This makes the model loader (
modelread.cpp) more careful with model files that contain bad or unexpected data. Problems that could crash the game while loading a damaged model now produce a warning instead, or the model is rejected cleanly.Why?
A damaged or hand-edited model file could crash the game during loading, or quietly read data from the wrong place. Valid models aren't affected: I checked every model in retail, MediaVPs 5.0.2 and the mods I have installed (7,966 POFs), and none of them hits the new checks except 12 mod models whose shield-mesh neighbor references now get reset (logged only, no dialog).
How?
Testing?
gr.loadModelin SM1-01 (MediaVPs 5.0.2, no other mods). Master crashed or failed a check on each; this branch loads them all, or rejects the broken one cleanly, with no reports.Anything Else?
Longer property strings are handled by #7863.
I used Claude Code to help find and test this.