fix(game): patch Desync DoS vulnerability with strict majority consensus

[berkelmali]

Resolves #3959

Description:

This PR fixes a Denial of Service (DoS) vulnerability in 1v1 matches related to desync reporting. The findOutOfSyncClients logic previously forced a game-ending desync if half or more players reported conflicting hashes (outOfSyncClients.length >= Math.floor(this.activeClients.length / 2)). In a 1v1, this meant a single malicious player sending a bad hash could trigger a global desync, crashing their opponent's game session.

The logic has been corrected to require a strict majority (> Math.floor(this.activeClients.length / 2)) to declare a lobby-wide desync. In a 1v1 game, a single malicious actor will now simply be flagged as the out-of-sync client and disconnected, allowing the honest player to continue their session uninterrupted.

Please complete the following:

• I have added screenshots for all UI updates
• I process any text displayed to the user through translateText() and I've added it to the en.json file
• I have added relevant tests to the test directory
• I confirm I have thoroughly tested these changes and take full responsibility for any bugs introduced

Please put your Discord username so you can be contacted if a bug or regression is found:

barfires

[coderabbitai]

Walkthrough

This PR refactors error handling in game completion and adjusts desynchronization detection logic. GameManager converts from sync try/catch to promise-based error handling for game.end(), and GameServer tightens the threshold for treating all active clients as out-of-sync from ≥50% to >50%.

Changes

Game Completion Error Handling

Layer / File(s)	Summary
Async error handling for game end src/server/GameManager.ts	When a game reaches the Finished phase, tick() now calls game.end() as a promise with an attached .catch() handler for error logging, instead of wrapping the call in a synchronous try/catch block. Errors are logged via the promise rejection path.

Desynchronization Detection Threshold

Layer / File(s)	Summary
Strict majority threshold for desync detection src/server/GameServer.ts	The findOutOfSyncClients method now forces all active clients to be treated as out-of-sync only when the number of detected out-of-sync clients is strictly greater than half (not equal to half), tightening the desync majority heuristic.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

• evanpelle

Poem

🎮 Games now finish with promise and grace,
Error logs catch what try-catch won't face.
Sync checks grow stricter, more fair than before,
Half turns to more—robustness at core. ⚙️

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: fixing a DoS vulnerability by implementing strict majority consensus in desync detection.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description check	✅ Passed	The pull request description clearly explains the DoS vulnerability fix, the specific logic change from >= to >, and the impact on 1v1 matches, directly relating to the code changes.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

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 and usage tips.}