Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[GEN][ZH] Fix heap-use-after-free in TeamInQueue::~TeamInQueue #469

Merged
merged 2 commits into from
Apr 2, 2025

Conversation

xezon
Copy link

@xezon xezon commented Mar 20, 2025

In PlayerList::reset, the TheTeamFactory is cleared. This will delete teams. But the players still keep a raw pointer to those teams and will use them on destruction. To avoid this trouble, we clean up the AIPlayer of the Player before we delete the teams.

There is a chance that this problem is related to the surrender mismatch, because here we have undefined behaviour before transfering units from one player to another player inside the team.

==18160==ERROR: AddressSanitizer: heap-use-after-free on address 0x4ef8a841 at pc 0x0022390e bp 0x016fe9fc sp 0x016fe9f0
READ of size 1 at 0x4ef8a841 thread T0
==18160==WARNING: Failed to use and restart external symbolizer!
    #0 0x0022390d in Team::setActive D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Include\Common\Team.h:312
    #1 0x00840efb in TeamInQueue::~TeamInQueue D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\GameLogic\AI\AIPlayer.cpp:3489
    #2 0x00841199 in TeamInQueue::`scalar deleting destructor'+0x29 (C:\Generals\English\Command & Conquer Generals Zero Hour\generals.exe+0xb51199)
    #3 0x001710e7 in MemoryPoolObject::deleteInstance D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Include\Common\GameMemory.h:761
    #4 0x00846b9b in AIPlayer::clearTeamsInQueue D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\GameLogic\AI\AIPlayer.cpp:449
    #5 0x00853d6b in AISkirmishPlayer::~AISkirmishPlayer D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\GameLogic\AI\AISkirmishPlayer.cpp:92
    #6 0x00853db9 in AISkirmishPlayer::`scalar deleting destructor'+0x29 (C:\Generals\English\Command & Conquer Generals Zero Hour\generals.exe+0xb63db9)
    #7 0x001710e7 in MemoryPoolObject::deleteInstance D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Include\Common\GameMemory.h:761
    #8 0x003bcb0b in Player::init D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\Common\RTS\Player.cpp:396
    #9 0x002278bd in PlayerList::init D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\Common\RTS\PlayerList.cpp:245
    #10 0x002283de in PlayerList::reset D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\Common\RTS\PlayerList.cpp:125
    #11 0x001a6523 in SubsystemInterfaceList::resetAll D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\Common\System\SubsystemInterface.cpp:190
    #12 0x00171775 in GameEngine::reset D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\Common\GameEngine.cpp:727
    #13 0x005948d3 in GameLogic::clearGameData D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\GameLogic\System\GameLogicDispatch.cpp:280
    #14 0x005956cc in GameLogic::logicMessageDispatcher D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\GameLogic\System\GameLogicDispatch.cpp:463
    #15 0x002c8fcc in GameLogic::processCommandList D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\GameLogic\System\GameLogic.cpp:2552
    #16 0x002cf181 in GameLogic::update D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\GameLogic\System\GameLogic.cpp:3693
    #17 0x00173bdd in GameEngine::update D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\Common\GameEngine.cpp:784
    #18 0x00946c7d in Win32GameEngine::update D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngineDevice\Source\Win32Device\Common\Win32GameEngine.cpp:90
    #19 0x001711fa in GameEngine::execute D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\Common\GameEngine.cpp:845
    #20 0x0015fc45 in GameMain D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\Common\GameMain.cpp:44
    #21 0x00158e77 in WinMain D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\Main\WinMain.cpp:1067
    #22 0x00e415f2 in __scrt_common_main_seh D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288
    #23 0x7569fcc8 in BaseThreadInitThunk+0x18 (C:\WINDOWS\System32\KERNEL32.DLL+0x6b81fcc8)
    #24 0x770682ad in RtlGetAppContainerNamedObjectPath+0x11d (C:\WINDOWS\SYSTEM32\ntdll.dll+0x4b2e82ad)
    #25 0x7706827d in RtlGetAppContainerNamedObjectPath+0xed (C:\WINDOWS\SYSTEM32\ntdll.dll+0x4b2e827d)

0x4ef8a841 is located 33 bytes inside of 92-byte region [0x4ef8a820,0x4ef8a87c)
freed by thread T0 here:
    #0 0x5369e6ed in free D:\a\_work\1\s\src\vctools\asan\llvm\compiler-rt\lib\asan\asan_malloc_win.cpp:125
    #1 0x536b081c in __asan::__RuntimeFunctions<__asan::Ucrtbase>::Free D:\a\_work\1\s\src\vctools\asan\llvm\compiler-rt\lib\asan\asan_win_runtime_functions.cpp:273
    #2 0x0015b610 in MemoryPool::freeBlock D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\Common\System\GameMemory.cpp:2033
    #3 0x001710f1 in MemoryPoolObject::deleteInstance D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Include\Common\GameMemory.h:762
    #4 0x00214dec in TeamPrototype::~TeamPrototype D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\Common\RTS\Team.cpp:845
    #5 0x00215ca9 in TeamPrototype::`scalar deleting destructor'+0x29 (C:\Generals\English\Command & Conquer Generals Zero Hour\generals.exe+0x525ca9)
    #6 0x0021aa35 in TeamFactory::clear D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\Common\RTS\Team.cpp:217
    #7 0x002283b1 in PlayerList::reset D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\Common\RTS\PlayerList.cpp:124
    #8 0x001a6523 in SubsystemInterfaceList::resetAll D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\Common\System\SubsystemInterface.cpp:190
    #9 0x00171775 in GameEngine::reset D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\Common\GameEngine.cpp:727
    #10 0x005948d3 in GameLogic::clearGameData D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\GameLogic\System\GameLogicDispatch.cpp:280
    #11 0x005956cc in GameLogic::logicMessageDispatcher D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\GameLogic\System\GameLogicDispatch.cpp:463
    #12 0x002c8fcc in GameLogic::processCommandList D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\GameLogic\System\GameLogic.cpp:2552
    #13 0x002cf181 in GameLogic::update D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\GameLogic\System\GameLogic.cpp:3693
    #14 0x00173bdd in GameEngine::update D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\Common\GameEngine.cpp:784
    #15 0x00946c7d in Win32GameEngine::update D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngineDevice\Source\Win32Device\Common\Win32GameEngine.cpp:90
    #16 0x001711fa in GameEngine::execute D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\Common\GameEngine.cpp:845
    #17 0x0015fc45 in GameMain D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\Common\GameMain.cpp:44
    #18 0x00158e77 in WinMain D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\Main\WinMain.cpp:1067
    #19 0x00e415f2 in __scrt_common_main_seh D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288
    #20 0x7569fcc8 in BaseThreadInitThunk+0x18 (C:\WINDOWS\System32\KERNEL32.DLL+0x6b81fcc8)
    #21 0x770682ad in RtlGetAppContainerNamedObjectPath+0x11d (C:\WINDOWS\SYSTEM32\ntdll.dll+0x4b2e82ad)
    #22 0x7706827d in RtlGetAppContainerNamedObjectPath+0xed (C:\WINDOWS\SYSTEM32\ntdll.dll+0x4b2e827d)

previously allocated by thread T0 here:
    #0 0x5369e7ed in malloc D:\a\_work\1\s\src\vctools\asan\llvm\compiler-rt\lib\asan\asan_malloc_win.cpp:134
    #1 0x0015ad82 in MemoryPool::allocateBlockImplementation D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\Common\System\GameMemory.cpp:2020
    #2 0x0021bacd in TeamFactory::createInactiveTeam D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\Common\RTS\Team.cpp:356
    #3 0x002216ef in TeamFactory::initTeam D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\Common\RTS\Team.cpp:250
    #4 0x00221495 in TeamFactory::initFromSides D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\Common\RTS\Team.cpp:234
    #5 0x00227d60 in PlayerList::newGame D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\Common\RTS\PlayerList.cpp:191
    #6 0x002cc8e0 in GameLogic::startNewGame D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\GameLogic\System\GameLogic.cpp:1492
    #7 0x002ceef8 in GameLogic::update D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\GameLogic\System\GameLogic.cpp:3596
    #8 0x00173bdd in GameEngine::update D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\Common\GameEngine.cpp:784
    #9 0x00946c7d in Win32GameEngine::update D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngineDevice\Source\Win32Device\Common\Win32GameEngine.cpp:90
    #10 0x001711fa in GameEngine::execute D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\Common\GameEngine.cpp:845
    #11 0x0015fc45 in GameMain D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\Common\GameMain.cpp:44
    #12 0x00158e77 in WinMain D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\Main\WinMain.cpp:1067
    #13 0x00e415f2 in __scrt_common_main_seh D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288
    #14 0x7569fcc8 in BaseThreadInitThunk+0x18 (C:\WINDOWS\System32\KERNEL32.DLL+0x6b81fcc8)
    #15 0x770682ad in RtlGetAppContainerNamedObjectPath+0x11d (C:\WINDOWS\SYSTEM32\ntdll.dll+0x4b2e82ad)
    #16 0x7706827d in RtlGetAppContainerNamedObjectPath+0xed (C:\WINDOWS\SYSTEM32\ntdll.dll+0x4b2e827d)

SUMMARY: AddressSanitizer: heap-use-after-free D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Include\Common\Team.h:312 in Team::setActive
Shadow bytes around the buggy address:
  0x4ef8a580: fa fa fa fa fd fd fd fd fd fd fd fd fd fd fd fd
  0x4ef8a600: fa fa fa fa fd fd fd fd fd fd fd fd fd fd fd fd
  0x4ef8a680: fa fa fa fa fd fd fd fd fd fd fd fd fd fd fd fd
  0x4ef8a700: fa fa fa fa fd fd fd fd fd fd fd fd fd fd fd fd
  0x4ef8a780: fa fa fa fa fd fd fd fd fd fd fd fd fd fd fd fd
=>0x4ef8a800: fa fa fa fa fd fd fd fd[fd]fd fd fd fd fd fd fd
  0x4ef8a880: fa fa fa fa fd fd fd fd fd fd fd fd fd fd fd fd
  0x4ef8a900: fa fa fa fa fd fd fd fd fd fd fd fd fd fd fd fd
  0x4ef8a980: fa fa fa fa fd fd fd fd fd fd fd fd fd fd fd fd
  0x4ef8aa00: fa fa fa fa fd fd fd fd fd fd fd fd fd fd fd fd
  0x4ef8aa80: fa fa fa fa fd fd fd fd fd fd fd fd fd fd fd fd
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
Address Sanitizer Error: Use of deallocated memory

@xezon xezon added Bug Something is not working right Major Severity: Minor < Major < Critical < Blocker ZeroHour Relates to Zero Hour labels Mar 20, 2025
@xezon
Copy link
Author

xezon commented Mar 23, 2025

Nabeel and Fatsam confirmed that the surrender mismatch still happens with this fix.

@Mauller Mauller force-pushed the xezon/fix-memory-teaminqueue branch from f24cd02 to 6655552 Compare March 30, 2025 10:47
@Mauller
Copy link

Mauller commented Mar 30, 2025

Fixed commits with relevant tags and added generals equivalent commit.

The delete player ai function does appear on a different line in in player.cpp but i made sure to place it in the same relative place to other functions around it. similar to the ZH commit.

@Mauller Mauller changed the title Fix: heap-use-after-free in TeamInQueue::~TeamInQueue [GEN][ZH] Fix: heap-use-after-free in TeamInQueue::~TeamInQueue Mar 30, 2025
@xezon xezon changed the title [GEN][ZH] Fix: heap-use-after-free in TeamInQueue::~TeamInQueue [GEN][ZH] Fix heap-use-after-free in TeamInQueue::~TeamInQueue Mar 30, 2025
@OmniBlade OmniBlade merged commit 4c4c94d into TheSuperHackers:main Apr 2, 2025
17 checks passed
@xezon xezon added this to the Code foundation build up milestone Apr 2, 2025
@xezon xezon deleted the xezon/fix-memory-teaminqueue branch April 2, 2025 15:57
@xezon xezon added the Stability Concerns stability of the runtime label Apr 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is not working right Major Severity: Minor < Major < Critical < Blocker Stability Concerns stability of the runtime ZeroHour Relates to Zero Hour
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants