Repository navigation
Harden campaign save loading against malformed data - #7906
Open
dustinspace217 wants to merge 5 commits into
Open
dustinspace217 wants to merge 5 commits into
dustinspace217 wants to merge 5 commits into
Conversation
Recent missions, mission records, the last flown ship and red alert weapons read from a campaign save are now checked against the lists they index, and the legacy save converter stays within its score tables. Bad entries are skipped with a warning; valid saves load as before.
The previous mission and loop reentry mission read from a campaign save are now checked as soon as they are read, and cleared with a log message when they are outside the campaign's mission list; a cleared loop reentry also leaves the loop. A negative next mission gets the same invalid-data handling as one past the end, and the ready room checks the previous mission against the mission count before using it. Valid saves load as before.
A section size of 0 turned the per-section read limit off, so the loader read whatever followed as that section's data. Such sections are only written empty, so they are now skipped. The read limit is also capped at the end of the file and checked before cfread shortens a read at the end of the file, so a section claiming more bytes than the file has stops with the existing limit warning instead of reading zeros. Only the pilot file loaders set a read limit. Valid saves load as before.
… file A section size larger than the bytes left could make the loader seek back to the same header forever. Both section loops now stop at such a section, and the rank-only loop also skips zero-size sections. The read limit is no longer capped at the file size (the size check above makes that cap unnecessary). Adds a unit test for the read limit at the end of a file.
BinaryFileHandler::nextSection now treats a section larger than the bytes left in the file like a zero-size one (Section::Invalid), so the section end can no longer wrap around and seek back into the file. The read limit unit test also checks a limit past the end of the file (short read, no exception).
BMagnu
added
cleanup
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
2 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.
What?
This makes the campaign save (
.csg) loader check the numbers it reads before using them as list positions.Why?
A damaged or hand-edited campaign save could crash the game while loading, or leave the previous pilot's data in place. Normal saves load exactly as before.
How?
Testing?
Anything Else?
The length of each name stored in the save is handled by #7863. I checked the legacy converter change by reading the code; I didn't have an old-format pilot to convert.
I used Claude Code to help find and test this.