[Caball009]

This pull request changes the second parameter for GameMessage::MSG_LOGIC_CRC to a separate type: GameMessage::MSG_LOGIC_CRC_PLAYBACK. The parameter is a boolean to determine if the message was created locally for replay playback.

It's wasteful to store this information in a parameter for a message type that's sent across the network and stored in replays, and can be easily replaced with a separate non-network type. The current situation also makes it possible for players to send true for playback during multiplayer games, causing replays of the match to report a mismatch when they shouldn't. This wouldn't be noticeable during the live game.

Despite the removal of the second message parameter, older clients can still parse newer replays because GameMessage::getArgument has a fallback option that returns 0 (or false) for missing arguments. See the code below.

Retail Generals code.

Retail Zero Hour code.

Current code.

TODO:

• Replicate to Generals.
• Restructure commits and add PR to commit titles.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

• Last reviewed commit: 48d6e6b243eac061a74f353803d9484a2a271563
• Latest commit, not reviewed: e725373

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 98a0b05f-e622-4469-819b-296407660b1d

📥 Commits

Reviewing files that changed from the base of the PR and between 47aa6e4e81d17268ddb2cbf3596e0649c4eaf940 and 48d6e6b243eac061a74f353803d9484a2a271563.

📒 Files selected for processing (2)

• Core/GameEngine/Include/Common/MessageStream.h
• Core/GameEngine/Source/GameLogic/System/GameLogicDispatch.cpp

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.

Walkthrough

The changes add a playback-specific logic CRC message. CRC generation, client filtering, game logic dispatch, and recorder culling now handle playback CRC messages separately from other logic CRC messages.

Changes

Replay CRC Handling

Layer / File(s)	Summary
Message contract and dispatch gates Core/GameEngine/Include/Common/MessageStream.h, Core/GameEngine/Source/Common/MessageStream.cpp, Core/GameEngine/Source/GameClient/MessageStream/CommandXlat.cpp, Core/GameEngine/Source/GameClient/GameClientDispatch.cpp, Core/GameEngine/Source/GameLogic/System/GameLogicDispatch.cpp	The message enum and string mapping include MSG_LOGIC_CRC_PLAYBACK. Client filtering preserves a specified set of messages when UI input is disabled. The game logic dispatcher handles MSG_CLEAR_GAME_DATA.
CRC generation and replay handling GeneralsMD/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp, GeneralsMD/Code/GameEngine/Include/GameLogic/GameLogic.h, Core/GameEngine/Source/GameLogic/System/GameLogicDispatch.cpp, GeneralsMD/Code/GameEngine/Source/Common/Recorder.cpp	Playback CRC generation appends MSG_LOGIC_CRC_PLAYBACK to TheCommandList; other CRCs use MSG_LOGIC_CRC through TheMessageStream. A playback handler passes the CRC to handleCRCMessage with playback enabled, and recorder culling removes network messages.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~12 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant GameLogic
  participant TheCommandList
  participant GameLogicDispatch
  participant handleCRCMessage
  GameLogic->>TheCommandList: Append MSG_LOGIC_CRC_PLAYBACK with CRC
  TheCommandList->>GameLogicDispatch: Dispatch playback CRC message
  GameLogicDispatch->>handleCRCMessage: Pass CRC with player index -1 and playback enabled

Loading

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to 4fcf1

The separation improves replay integrity, but default compatibility builds still allow a match participant to send the new replay-only message and repeatedly accumulate memory on receiving clients. Builds enforcing network-only message types block this path.

Retained concerns

• Medium · security · inferred: In builds retaining RETAIL_COMPATIBLE_CRC=1, a malicious match participant can supply MSG_LOGIC_CRC_PLAYBACK through the existing network-command ingress exemption. The new handler treats it as locally generated playback data without checking replay mode or network activity and appends its CRC to recorder-owned state. Repetition can accumulate queued CRCs during multiplayer, where ordinary CRC handling updates the network cache rather than consuming this queue, creating a potential client memory-exhaustion path. The ingress exemption is pre-existing; the newly reachable playback-state mutation is introduced by this PR. Compatibility-OFF builds reject this message before dispatch.

Security review details

Security Blast Radius

• inferred — The identified attack scope is receiving game clients in a shared multiplayer match running compatibility-enabled builds. A malicious participant supplying otherwise accepted game commands can target recorder state inside those client processes; the demonstrated source path does not establish unauthenticated internet reachability or access to external services.

Security Findings and Attack Paths

• inferred — An attacker-controlled wire type can become a playback-only game message through the compatibility-enabled relay branch, then reach unconditional CRC queue insertion. Repeating this path during multiplayer can retain allocations across frames because normal network CRC handling does not drain that queue. The source supports potential resource exhaustion, not a measured exhaustion rate or demonstrated crash.

Trust Boundaries and Controls

• observed — Being outside the network enum range is enforced as an ingress boundary only when retail CRC compatibility is disabled. That branch rejects the new playback type. Network-created messages carry a participant's player identity, but a valid player identity alone does not establish local playback provenance. Normal replay recording independently excludes the playback-only type.

Resilience and Maintainability Implications

• observed — Per-frame command destruction removes message objects but does not undo CRC values already appended to the recorder FIFO. Queue insertion skips the initial multiplayer CRC when required, then appends without a size limit or frame identity. Replay startup reconstructs the queue, but that recovery does not prevent accumulation during an active multiplayer session.

Hardening Proposals

• proposed — A replay-mode and no-active-network guard on playback-only handling would make the local-playback authority boundary independent of CRC compatibility settings. This is a proposed control, not an existing safeguard.

Pre-merge checks: 4

Check name	Status	Explanation
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Title check		The title clearly and concisely describes the main change: adding a separate game message type for playback CRC.
Description check		The description is directly related to the changeset. It explains the replacement of the playback boolean with MSG_LOGIC_CRC_PLAYBACK, the motivation, compatibility behavior, and implementation scope.

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[greptile-apps]

Confidence Score: 5/5

[Medium impact] The PR appears safe to merge, with the missing-checksum finding fixed and no new actionable issues found.

Summary

Separates locally generated replay checksums into MSG_LOGIC_CRC_PLAYBACK in both game variants.

• Replay CRCs use a separate message from multiplayer CRCs.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Calculate checksum] --> B{Replay playback?}
  B -- No --> C[MSG_LOGIC_CRC]
  B -- Yes --> D[MSG_LOGIC_CRC_PLAYBACK]
  C --> E[Live checks or replay recording]
  D --> F{Simulation playback?}
  F -- No --> G[MessageStream]
  F -- Yes --> H[CommandList]
  G --> H
  H --> I[Queue local replay checksum]
  J[Recorded MSG_LOGIC_CRC] --> K[Compare recorded and local checksums]
  I --> K

Loading

_{Reviews (5) · Last reviewed commit: "tweak(gamemessage): Add separate game me..." · Reviewed by Greptile}

[xezon]

The second commit mixes changes with refactors. It would be ideal to move refactors into commit 1. Otherwise looks ok.

[Caball009]

The second commit mixes changes with refactors. It would be ideal to move refactors into commit 1.

Yeah, I can change that. Do you mind if I keep commit 1 as is, and separate commit 2 into a refactor commit and a change commit?

[xezon]

Yeah, I can change that. Do you mind if I keep commit 1 as is, and separate commit 2 into a refactor commit and a change commit?

Do as you see fit.

[xezon]

Logically looks to be proper.

[xezon]

Pull id needs to be added to commit titles.

[Caball009]

Pull id needs to be added to commit titles.

You want to rebase and merge? In that case it might make more sense to combine the refactor commits into one.

[xezon]

It makes sense to have 1 commit for refactors and 1 for the change.

[Caball009]

I expect to update it sometime in the next 24h.