Skip to content

Add default case to all savegame tag switches to prevent parser desync - #1233

Open
acato wants to merge 1 commit into
We-the-People-civ4col-mod:developfrom
acato:fix/savegame-missing-default-case
Open

Add default case to all savegame tag switches to prevent parser desync#1233
acato wants to merge 1 commit into
We-the-People-civ4col-mod:developfrom
acato:fix/savegame-missing-default-case

Conversation

@acato

@acato acato commented Apr 3, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Adds default: bContinue = false; break; to 19 savegame switch statements that had no default case
  • Adds bContinue = false; to 3 existing default cases that only had FAssert (stripped in Release builds)
  • Without this, an unrecognized tag value leaves its data bytes unconsumed, permanently misaligning the parser — a crafted savegame exploits this to inject controlled values into arbitrary game state fields

22 files touched — all *Savegame.cpp files with tag-reading switch statements.

Fixes #1230

Test plan

  • Load an existing savegame — verify it loads identically to before (no false positives from the default case)
  • Save and reload mid-game — verify no data loss
  • In Assert build: verify no FAssert fires during normal save/load (confirms no legitimate tag hits the default)
  • Confirm the fix compiles cleanly in Assert, Release, and FinalRelease targets

🤖 Generated with Claude Code

When an unrecognized tag value is encountered during savegame
deserialization, the missing default case meant the tag's data
bytes were not consumed. The next loop iteration read those data
bytes as the next tag identifier, permanently misaligning the
parser. A crafted savegame exploits this to inject controlled
values into arbitrary game state fields.

Setting bContinue = false in the default case stops deserialization
safely. Files that already had FAssert-only defaults (which are
stripped in Release builds) now also set bContinue = false.

Fixes We-the-People-civ4col-mod#1230

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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.

Security: Missing default case in savegame tag switches enables parser desync exploit

1 participant