Contained unit dereferences its destroyed container during combat after a surrender with asset transfer

[bobtista]

Split out of #3315, which covers the crash when such a unit is destroyed.

After a GLA player surrenders with units inside a Tunnel Network in a Generals network game, the tunnels and occupants go to a living ally. Once the transferred tunnels are sold or destroyed, the occupants keep a m_containedBy pointer to the freed tunnel while still behaving as contained. Code that follows that pointer during play reads whatever now occupies the memory.

Observed crash, both clients on the same frame, in an automated LAN game recorded on a build of main without freed-memory poisoning, about two minutes after the surrender:

AIAttackAimAtTargetState::onEnter   AIStates.cpp:4828
StateMachine::internalSetState
StateMachine::initDefaultState
AIAttackState::onEnter               AIStates.cpp:5377
AIStateMachine::setState
AIUpdateInterface::privateAttackObject
AIUpdateInterface::aiDoCommand
AIIdleState::update                  AIStates.cpp:1443
AIUpdateInterface::update
GameLogic::update

The unit enters its attack state and asks its supposed container for a firing position. source->getContainedBy() is the freed tunnel's address, getContain() on the object that reused it returns null, and attemptBestFirePointPosition() is called on that. Headless playback of the recording does not reproduce the crash because the memory reuse differs from the live game; instrumentation on playback still shows the stale pointers. Recording: surrender-tunnel-ally-21900.rep.

Caball009 reports the same class in Weapon::calcProjectileLaunchPosition, which dereferences launcher->getContainedBy()->getContain() when a contained unit fires.

Three further automated games on a build with freed-memory poisoning all died on the same read, about 100 frames after the surrender:

Object::isAbleToAttack               Object.cpp:2975
AI::findClosestEnemy
...
AIUpdateInterface::update
GameLogic::update

containedBy->getContain()->isPassengerAllowedToFire() through the stale pointer. This line already null checks getContain(); the crash is a non-null garbage module pointer read from the freed block (0xDEADBEEF under poisoning), whose virtual call faults. Without poisoning the same line reads whatever reused the memory: a null module, which the check tolerates, an unrelated live object's module, or the dead tunnel's intact module. Six such games were run: four crashed there; in one the surrendering player had no units inside a tunnel, in the other the transferred tunnels were never destroyed before the match ended, so neither had a stale pointer to hit.

Sweep of every getContainedBy() use in the Generals simulation code (about 60 in 18 files; client code such as SelectionInfo.cpp and InGameUI.cpp is out of scope here). After dropping null tests, pointer compares and debug strings, these dereference a contained unit's container, listed in the order a hidden, AI driven unit would reach them. Note that a null check on the module only covers memory reused by an object without a contain module; a freed block, poisoned or intact, and a block reused by another container pass such checks.

site	dereference	when
Object.cpp:2975 Object::isAbleToAttack	getContain()->isPassengerAllowedToFire(), module null checked	every idle target scan (observed, four games)
AIUpdate.cpp:4315	getContain()->isPassengerAllowedToFire(), module not null checked	weapon slot selection
AIStates.cpp:4825 AIAttackAimAtTargetState::onEnter	getContain()->attemptBestFirePointPosition	entering the attack (observed)
Weapon.cpp:2833 Weapon::calcProjectileLaunchPosition	getContain()->isEnclosingContainerFor	firing (reported by Caball009)
WeaponSet.cpp:652	getContain(), then null checked	range check
AIUpdate.cpp:4368	getGeometryInfo() of the container	target search radius
AIPathfind.cpp:9300	getID() of the container	pathfinding
AIUpdate.cpp:3676	container handed to the exit path	exit order
AIStates.cpp:1121, AIStates.cpp:1209	isKindOf, isAirborneTarget of the container	attack approach
StealthUpdate.cpp:540, StealthDetectorUpdate.cpp:159	getContain(), null checked	stealth units
PhysicsUpdate.cpp:1216	container chain to getPosition()	collisions; the unit is out of the partition, so unlikely
WeaponSet.cpp:567, AIStates.cpp:439	the victim's container	only if such a unit is targeted; it is hidden

Team.cpp, TunnelTracker.cpp, OpenContain.cpp (except the debug string), AITNGuard.cpp and RailroadGuideAIUpdate.cpp compare or null test the pointer without dereferencing it. With the freed block intact, isAbleToAttack calls the dead tunnel's isPassengerAllowedToFire() through its old vtable; what happens after reuse depends on what occupies the memory, so the later sites are reachable and the outcome is not defined by the code.

Zero Hour's AIAttackAimAtTargetState::onEnter already null-checks the contain module, so this exact null read needs the reused block to carry a contain module there, but the stale pointer is still dereferenced. Zero Hour can reach a stale m_containedBy through the destroyed Troop Crawler path in #2467, so it is potentially affected; the confirmed reproduction is Generals.

Reproduction

1. Start a Generals LAN team game as GLA with a living ally. Skirmish does not offer Surrender.
2. Build a Tunnel Network and place units inside it.
3. Surrender, transferring assets to the ally.
4. Have the transferred tunnels sold or destroyed.
5. Keep playing so the transferred units fight.

The game may crash during combat. Reproduction depends on memory reuse.

Related: #3315, #3308, #3242, #3165, #2467.