Skip to content

fix: Prevent PlayerList::newGame array out-of-bounds and null dereferences - #544

Open
seer-by-sentry[bot] wants to merge 1 commit into
mainfrom
seer/fix/playerlist-array-bounds
Open

fix: Prevent PlayerList::newGame array out-of-bounds and null dereferences#544
seer-by-sentry[bot] wants to merge 1 commit into
mainfrom
seer/fix/playerlist-array-bounds

Conversation

@seer-by-sentry

@seer-by-sentry seer-by-sentry Bot commented Aug 1, 2026

Copy link
Copy Markdown

This PR addresses a critical crash in Player::init that occurred when loading maps with more player sides than MAX_PLAYER_COUNT.

Root Cause:
In PlayerList::newGame:

  1. The first loop iterates through map sides and attempts to initialize players using m_players[m_playerCount++]. If TheSidesList->getNumSides() exceeds MAX_PLAYER_COUNT, m_playerCount goes out of bounds, leading to a garbage pointer being assigned to p. Subsequent call to p->initFromDict(d) then causes an EXCEPTION_ACCESS_VIOLATION_READ.
  2. The second loop attempts to find players using findPlayerWithNameKey. For players that were skipped in the first loop (due to exceeding MAX_PLAYER_COUNT or having an empty name key), findPlayerWithNameKey returns nullptr. Subsequent calls to p->setPlayerRelationship() or p->setDefaultTeam() on this nullptr result in a null dereference crash.

Solution:

  1. First loop: Added a bounds check if (m_playerCount >= MAX_PLAYER_COUNT) before accessing m_players[m_playerCount++]. If the limit is reached, the side is skipped, and a DEBUG_ASSERTCRASH is triggered to alert developers about oversized maps.
  2. Second loop: Added a null check if (!p) after findPlayerWithNameKey to skip processing for players that were not successfully initialized (either due to being skipped in the first loop or being neutral with an empty name key).

Fixes CLIENT-3AP

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.

0 participants