Skip to content

Fix OOB read from unterminated player names in save/network data - #8651

Merged
StephenCWills merged 1 commit into
diasurgical:masterfrom
romanstingler:fix/unterminated-player-names
Jul 27, 2026
Merged

Fix OOB read from unterminated player names in save/network data#8651
StephenCWills merged 1 commit into
diasurgical:masterfrom
romanstingler:fix/unterminated-player-names

Conversation

@romanstingler

Copy link
Copy Markdown
Contributor

Replaces unbounded string operations with explicitly bounded std::string_view construction, following the security audit finding in SECURITY_AUDIT.md (section D).

Possible with a crafted save file or malicious network packet

PlayerPack::pName and PlayerNetPack::pName are char[32] fields populated via memcpy from raw save archives or network data. If a crafted save file (or malicious peer in multiplayer) fills all 32 bytes with non-null characters, the name has no null terminator within the buffer.

pfile.cpp:706 used strcpy(hero_names[i], pkplr.pName), which scans indefinitely for a null byte, causing an out-of-bounds read on the source and a potential out-of-bounds write past the 32-byte destination.

pack.cpp:370 and pack.cpp:468 (UnPackPlayer, UnPackNetPlayer) passed packed.pName to CopyUtf8, which accepts a std::string_view. The implicit char[32]std::string_view conversion calls strlen() internally, triggering the same unbounded scan before CopyUtf8 ever executes.

@StephenCWills
StephenCWills merged commit b4dfc8d into diasurgical:master Jul 27, 2026
25 checks passed
@qndel

qndel commented Jul 27, 2026

Copy link
Copy Markdown
Member

Replaces unbounded string operations with explicitly bounded std::string_view construction, following the security audit finding in SECURITY_AUDIT.md (section D).

What

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.

3 participants