GameWindow: use-after-free on exiting the score overview window prematurely

[Caball009]

Prerequisites

• I have searched for similar issues and confirmed this is not a duplicate

Game Version

• Command & Conquer Generals
• Command & Conquer Generals: Zero Hour
• Other (please specify below)

Bug Description

I didn't test with Generals, but I assume it's exactly the same.

When exiting the score overview window before it's finished drawing, some (or all) of the GameWindow instances are deleted (deleteInstance), but still drawn apparently, which triggers the address sanitizer here:

GeneralsGameCode/GeneralsMD/Code/GameEngine/Include/GameClient/GameWindow.h

Line 274 in 7d884ad

const Image *winGetEnabledImage( Int index ) { return m_instData.m_enabledDrawData[ index ].image; }

Reproduction Steps

1. Start a new skirmish game.
2. Exit match.
3. Don't let the score overview finish but exit to main screen.
4. No visual sign but address sanitizer gets triggered because of use-after-free.

Additional Context

I was using windowed mode.

[Mauller]

Call stack for this, just having a quick look at it.

>	generalszh.exe!GameWindow::winGetEnabledImage(int index) Line 274	C++
 	generalszh.exe!ScoreScaleUpTransition::draw() Line 980	C++
 	generalszh.exe!TransitionWindow::draw() Line 218	C++
 	generalszh.exe!TransitionGroup::draw() Line 340	C++
 	generalszh.exe!GameWindowTransitionsHandler::draw() Line 442	C++
 	generalszh.exe!GameWindowManager::winRepaint() Line 1366	C++
 	generalszh.exe!W3DInGameUI::draw() Line 439	C++
 	generalszh.exe!SubsystemInterface::DRAW() Line 105	C++
 	generalszh.exe!W3DDisplay::draw() Line 1910	C++
 	generalszh.exe!SubsystemInterface::DRAW() Line 105	C++
 	generalszh.exe!GameClient::update() Line 762	C++
 	generalszh.exe!W3DGameClient::update() Line 94	C++
 	generalszh.exe!SubsystemInterface::UPDATE() Line 79	C++
 	generalszh.exe!GameEngine::update() Line 735	C++
 	generalszh.exe!Win32GameEngine::update() Line 90	C++
 	generalszh.exe!GameEngine::execute() Line 809	C++
 	generalszh.exe!GameMain(int argc, char * * argv) Line 44	C++

[Mauller]

Have this one sorted, the issue was that the transition could be marked as ended causing resources to be unloaded, but the draw update had an early return in the wrong order with a function that was attempting to retrieve a resource which could be freed at this point.

[xezon]

Asan report:

==19720==ERROR: AddressSanitizer: heap-use-after-free on address 0x4ca08cc4 at pc 0x01856057 bp 0x00afe8b4 sp 0x00afe8a8
READ of size 4 at 0x4ca08cc4 thread T0
==19720==WARNING: Failed to use and restart external symbolizer!
    #0 0x01856056 in GameWindow::winGetEnabledImage D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Include\GameClient\GameWindow.h:274
    #1 0x0184d6dc in ScoreScaleUpTransition::draw D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\GameClient\GUI\GameWindowTransitionsStyles.cpp:980
    #2 0x011a7588 in TransitionWindow::draw D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\GameClient\GUI\GameWindowTransitions.cpp:218
    #3 0x011a8a1b in TransitionGroup::draw D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\GameClient\GUI\GameWindowTransitions.cpp:339
    #4 0x011a9bb3 in GameWindowTransitionsHandler::draw D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\GameClient\GUI\GameWindowTransitions.cpp:442
    #5 0x0107ac60 in GameWindowManager::winRepaint D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\GameClient\GUI\GameWindowManager.cpp:1366
    #6 0x01b8e90f in W3DInGameUI::draw D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngineDevice\Source\W3DDevice\GameClient\W3DInGameUI.cpp:439
    #7 0x01042fa0 in SubsystemInterface::DRAW D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Include\Common\SubsystemInterface.h:135
    #8 0x0196836d in W3DDisplay::draw D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngineDevice\Source\W3DDevice\GameClient\W3DDisplay.cpp:1910
    #9 0x01042fa0 in SubsystemInterface::DRAW D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Include\Common\SubsystemInterface.h:135
    #10 0x0103a20f in GameClient::update D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\GameClient\GameClient.cpp:762
    #11 0x01a389ee in W3DGameClient::update D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngineDevice\Source\W3DDevice\GameClient\W3DGameClient.cpp:94
    #12 0x00d1edb0 in SubsystemInterface::UPDATE D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Include\Common\SubsystemInterface.h:134
    #13 0x00d15d2c in GameEngine::update D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\Common\GameEngine.cpp:734
    #14 0x01962063 in Win32GameEngine::update D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngineDevice\Source\Win32Device\Common\Win32GameEngine.cpp:90
    #15 0x00d15f34 in GameEngine::execute D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\Common\GameEngine.cpp:809
    #16 0x00cf49c6 in GameMain D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\Common\GameMain.cpp:44
    #17 0x00cea770 in WinMain D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\Main\WinMain.cpp:1025
    #18 0x0208b0ec in invoke_main D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:102
    #19 0x0208aff9 in __scrt_common_main_seh D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288
    #20 0x0208ae9c in __scrt_common_main D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:330
    #21 0x0208b157 in WinMainCRTStartup D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_winmain.cpp:16
    #22 0x762afcc8 in BaseThreadInitThunk+0x18 (C:\WINDOWS\System32\KERNEL32.DLL+0x6b81fcc8)
    #23 0x77d982ad in RtlGetAppContainerNamedObjectPath+0x11d (C:\WINDOWS\SYSTEM32\ntdll.dll+0x4b2e82ad)
    #24 0x77d9827d in RtlGetAppContainerNamedObjectPath+0xed (C:\WINDOWS\SYSTEM32\ntdll.dll+0x4b2e827d)

0x4ca08cc4 is located 68 bytes inside of 728-byte region [0x4ca08c80,0x4ca08f58)
freed by thread T0 here:
    #0 0x5403e6ed in free D:\a\_work\1\s\src\vctools\asan\llvm\compiler-rt\lib\asan\asan_malloc_win.cpp:125
    #1 0x5405087c in __asan::__RuntimeFunctions<__asan::Ucrtbased>::Free D:\a\_work\1\s\src\vctools\asan\llvm\compiler-rt\lib\asan\asan_win_runtime_functions.cpp:273
    #2 0x00cf16cc in operator delete D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\Common\System\GameMemoryNull.cpp:167
    #3 0x0208a093 in operator delete D:\a\_work\1\s\src\vctools\asan\llvm\compiler-rt\lib\asan\asan_win_delete_scalar_size_thunk.cpp:44
    #4 0x01bb5cd4 in W3DGameWindow::`scalar deleting destructor'+0x24 (C:\Generals\English\Command & Conquer Generals Zero Hour\generalszh.exe+0x1335cd4)
    #5 0x00d2072e in MemoryPoolObject::deleteInstance D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Include\Common\GameMemoryNull.h:113
    #6 0x0108240f in GameWindowManager::processDestroyList D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\GameClient\GUI\GameWindowManager.cpp:125
    #7 0x0106aa30 in GameWindowManager::update D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\GameClient\GUI\GameWindowManager.cpp:247
    #8 0x00d1edb0 in SubsystemInterface::UPDATE D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Include\Common\SubsystemInterface.h:134
    #9 0x01039e90 in GameClient::update D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\GameClient\GameClient.cpp:640
    #10 0x01a389ee in W3DGameClient::update D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngineDevice\Source\W3DDevice\GameClient\W3DGameClient.cpp:94
    #11 0x00d1edb0 in SubsystemInterface::UPDATE D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Include\Common\SubsystemInterface.h:134
    #12 0x00d15d2c in GameEngine::update D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\Common\GameEngine.cpp:734
    #13 0x01962063 in Win32GameEngine::update D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngineDevice\Source\Win32Device\Common\Win32GameEngine.cpp:90
    #14 0x00d15f34 in GameEngine::execute D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\Common\GameEngine.cpp:809
    #15 0x00cf49c6 in GameMain D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\Common\GameMain.cpp:44
    #16 0x00cea770 in WinMain D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\Main\WinMain.cpp:1025
    #17 0x0208b0ec in invoke_main D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:102
    #18 0x0208aff9 in __scrt_common_main_seh D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288
    #19 0x0208ae9c in __scrt_common_main D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:330
    #20 0x0208b157 in WinMainCRTStartup D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_winmain.cpp:16
    #21 0x762afcc8 in BaseThreadInitThunk+0x18 (C:\WINDOWS\System32\KERNEL32.DLL+0x6b81fcc8)
    #22 0x77d982ad in RtlGetAppContainerNamedObjectPath+0x11d (C:\WINDOWS\SYSTEM32\ntdll.dll+0x4b2e82ad)
    #23 0x77d9827d in RtlGetAppContainerNamedObjectPath+0xed (C:\WINDOWS\SYSTEM32\ntdll.dll+0x4b2e827d)

previously allocated by thread T0 here:
    #0 0x5403e7ed in malloc D:\a\_work\1\s\src\vctools\asan\llvm\compiler-rt\lib\asan\asan_malloc_win.cpp:134
    #1 0x00cf160e in operator new D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\Common\System\GameMemoryNull.cpp:158
    #2 0x01b97b81 in W3DGameWindowManager::allocateNewWindow D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngineDevice\Include\W3DDevice\GameClient\W3DGameWindowManager.h:86
    #3 0x010799ea in GameWindowManager::winCreate D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\GameClient\GUI\GameWindowManager.cpp:1403
    #4 0x013dbdf7 in createWindow D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\GameClient\GUI\GameWindowManagerScript.cpp:2047
    #5 0x013d47af in parseWindow D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\GameClient\GUI\GameWindowManagerScript.cpp:2455
    #6 0x013dc769 in parseChildWindows D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\GameClient\GUI\GameWindowManagerScript.cpp:2234
    #7 0x013d48ff in parseWindow D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\GameClient\GUI\GameWindowManagerScript.cpp:2475
    #8 0x013dc769 in parseChildWindows D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\GameClient\GUI\GameWindowManagerScript.cpp:2234
    #9 0x013d48ff in parseWindow D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\GameClient\GUI\GameWindowManagerScript.cpp:2475
    #10 0x013d3c35 in GameWindowManager::winCreateFromScript D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\GameClient\GUI\GameWindowManagerScript.cpp:2869
    #11 0x00e9f315 in WindowLayout::load D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\GameClient\GUI\WindowLayout.cpp:216
    #12 0x013d3dd0 in GameWindowManager::winCreateLayout D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\GameClient\GUI\GameWindowManagerScript.cpp:2660
    #13 0x0147951f in Shell::doPush D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\GameClient\GUI\Shell\Shell.cpp:614
    #14 0x014780d4 in Shell::shutdownComplete D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\GameClient\GUI\Shell\Shell.cpp:687
    #15 0x01477826 in Shell::push D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\GameClient\GUI\Shell\Shell.cpp:316
    #16 0x013a11ba in GameLogic::clearGameData D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\GameLogic\System\GameLogicDispatch.cpp:272
    #17 0x0139c84c in GameLogic::logicMessageDispatcher D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\GameLogic\System\GameLogicDispatch.cpp:463
    #18 0x00f30517 in GameLogic::processCommandList D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\GameLogic\System\GameLogic.cpp:2562
    #19 0x00f30035 in GameLogic::update D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\GameLogic\System\GameLogic.cpp:3703
    #20 0x00d1edb0 in SubsystemInterface::UPDATE D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Include\Common\SubsystemInterface.h:134
    #21 0x00d15de0 in GameEngine::update D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\Common\GameEngine.cpp:748
    #22 0x01962063 in Win32GameEngine::update D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngineDevice\Source\Win32Device\Common\Win32GameEngine.cpp:90
    #23 0x00d15f34 in GameEngine::execute D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\Common\GameEngine.cpp:809
    #24 0x00cf49c6 in GameMain D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Source\Common\GameMain.cpp:44
    #25 0x00cea770 in WinMain D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\Main\WinMain.cpp:1025
    #26 0x0208b0ec in invoke_main D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:102
    #27 0x0208aff9 in __scrt_common_main_seh D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288

SUMMARY: AddressSanitizer: heap-use-after-free D:\Projects\TheSuperHackers\GeneralsGameCode\GeneralsMD\Code\GameEngine\Include\GameClient\GameWindow.h:274 in GameWindow::winGetEnabledImage
Shadow bytes around the buggy address:
  0x4ca08a00: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x4ca08a80: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x4ca08b00: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x4ca08b80: fd fd fd fd fd fd fd fd fd fd fd fa fa fa fa fa
  0x4ca08c00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x4ca08c80: fd fd fd fd fd fd fd fd[fd]fd fd fd fd fd fd fd
  0x4ca08d00: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x4ca08d80: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x4ca08e00: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x4ca08e80: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x4ca08f00: fd fd fd fd fd fd fd fd fd fd fd fa fa fa fa fa
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

[Mauller]

So yeah it's the call to retrieve the image causing the issue as the game window no longer exists by the looks of things.