Cleanup of scripted players on mod maps can cause a read access violation in Team::KillTeam()

[Mauller]

If a player is used within scripts that is not fully initialised, due to not being an AI or human player, the cleanup of this player can cause an access violation due to the player not having a player template related to that player.

This occurs in ScriptActions::doPlayerKill when KillTeam is called within KillPlayer. Team::killTeam() attempts to obtain the player template so it can retrieve the players beacon template. This is done so any player placed beacons can be cleaned up.

As the normal codepath does not perform a null pointer check at this point, the code attempts to call a function on a null m_playerTemplate

This seems to be more of a problem when scripts are used, or possibly misused.

The offending line in Team::killTeam()

	const ThingTemplate *beaconTemplate = TheThingFactory->findTemplate( getControllingPlayer()->getPlayerTemplate()->getBeaconTemplate() );

The callstack:

 	generalszh.exe!AsciiString::AsciiString(const AsciiString & stringSrc) Line 87	C++
 	generalszh.exe!PlayerTemplate::getBeaconTemplate() Line 109	C++
>	generalszh.exe!Team::killTeam() Line 2447	C++
 	generalszh.exe!Player::killPlayer() Line 2080	C++
 	generalszh.exe!ScriptActions::doPlayerKill(const AsciiString & playerName) Line 2541	C++
 	generalszh.exe!ScriptActions::executeAction(ScriptAction * pAction) Line 6746	C++
 	generalszh.exe!ScriptEngine::executeActions(ScriptAction * pActionHead) Line 7639	C++
 	generalszh.exe!ScriptEngine::executeScript(Script * pScript) Line 7027	C++
 	generalszh.exe!ScriptEngine::executeScripts(Script * pScriptHead) Line 7686	C++
 	generalszh.exe!ScriptEngine::update() Line 5592	C++
 	generalszh.exe!SubsystemInterface::UPDATE() Line 78	C++
 	generalszh.exe!GameLogic::update() Line 3651	C++
 	generalszh.exe!SubsystemInterface::UPDATE() Line 78	C++
 	generalszh.exe!GameEngine::update() Line 754	C++
 	generalszh.exe!Win32GameEngine::update() Line 90	C++
 	generalszh.exe!GameEngine::execute() Line 815	C++
 	generalszh.exe!GameMain(int argc, char * * argv) Line 44	C++
 	generalszh.exe!WinMain(HINSTANCE__ * hInstance, HINSTANCE__ * hPrevInstance, char * lpCmdLine, int nCmdShow) Line 986	C++

The map which can trigger this behaviour:
[AOD] Versus AOD v077.zip

To play the map you need to set a hard AI in the fifth player slot and set yourself and the AI as the GLA faction. Some instructions are included with the map.

To trigger the issue, simply let yourself lose the game by losing all ten lives. It does not need any other players and can be triggered in a skirmish game.

This is likely a script issue overall with how the players are being used, but we can work around the problem by adding a null pointer check when retrieving the player template. subsequent code already checks for null on retrieving the beacon template.

Players also only get fully initialised when they are full AI or Human players. So i see this as a mod scripting issue.

[Mauller]

As a side note, people running Gentool don't appear to have this issue occur for them, so something Gentool does appears to be bypasing the crash.

[Mauller]

Something else noted, ScriptActions::doPlayerKill(const AsciiString & playerName) retrieves the player based on the passed name, but the playerName passed is an empty string and does not relate to any ingame player.