bugfix(object): Avoid crash with dangling contain module in Object::onDestroy() when Troop Crawler is destroyed while en route to Reinforcement Pad by Caball009 · Pull Request #3165 · TheSuperHackers/GeneralsGameCode

Caball009 · 2026-08-18T01:42:58Z

Follow up for bugfix(object): Avoid crash with dangling contain module in Object::onDestroy() when Reinforcement Pad is destroyed before Troop Crawler drop #2747
Follow up for bugfix(contain): Restore retail compatibility after changes to Object::m_containedByID #2868
Fixes issue Destroyed Troop Crawler from reinforcement pad may crash the game due to use-after-free bug #2467

The purpose of the two pull requests was to avoid a crash while retaining retail compatibility, but the fix is rather convoluted. Additionally, this is the second time I came across one or more replays that mismatch because of those changes. So the purpose of this PR is to revert the previous two and fix the crash with a different implementation.

Here's a replay that mismatches with the current implementation. It mismatches because it relied on use-after-free bugs, which was intentionally prevented to avoid potential crashes:
18-11-57_2v4_0_24_HardAI_HardAI_HardAI_HardAI.zip

If a troop crawler that's en route (by plane) to the reinforcement pad gets destroyed, the occupants are left in a state of limbo (see issue). They hold on to the pointer of the destroyed troop crawler, which leads to use-after-free bugs. When the occupants' objects eventually get destroyed (e.g. when a player surrenders or the game ends), the use-after-free bug may crash the game when OpenContain::m_containList is accessed after its destruction.

GeneralsGameCode/GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp

Line 437 in a40cdb6

    
           ContainedItemsList::iterator it = std::find(m_containList.begin(), m_containList.end(), rider);

Here's a minimal crash reproduction that shows why the crash happens during the call to std::find:
https://godbolt.org/z/znjGGrTvG
https://godbolt.org/z/MzYz6eeMb

See commits for cleaner diff.

TODO:

Replicate to Generals.

…o Object::m_containedByID (TheSuperHackers#2868)" This reverts commit a20992d.

…bject::onDestroy() when Reinforcement Pad is destroyed before Troop Crawler drop (TheSuperHackers#2747)" This reverts commit df2224b.

qodo-free-for-open-source-projects · 2026-08-18T03:20:44Z

PR Summary by Qodo

Prevent crash from dangling containment modules during destruction

🐞 Bug fix 🕐 20-40 Minutes

AI Description

• Prevents destruction crashes when occupants retain a destroyed reinforcement transport container.
• Guards retail-compatible containment removal before traversing a destructed occupant list.
• Reverts container-ID tracking changes that caused replay mismatches.

Diagram

graph TD
  A["Container destroyed"] -->|calls| B["OpenContain destructor"] -->|retail mode| C["Drain occupant list"] -->|later| D["Occupant destroyed"] -->|calls removal| E{"List empty?"}
  E -->|Yes| F["Skip list search"]
  E -->|No| G["Remove occupant"]

High-Level Assessment

The following are alternative approaches to this PR:

1. Clear occupant links eagerly

➕ Eliminates dangling container pointers at their source.
➕ Provides conventional and safer object-lifetime semantics.
➖ Changes simulation behavior and can mismatch retail-compatible replays.
➖ Requires reliable enumeration and cleanup of every affected occupant.

2. Validate containers by ObjectID

➕ Avoids dereferencing a container after it leaves the object registry.
➕ Makes pointer validity explicit during occupant destruction.
➖ The prior implementation was complex and caused replay mismatches.
➖ Requires synchronized pointer and ID handling across containment implementations.

3. Retain containment modules longer

➕ Keeps module storage valid until dependent occupants are cleaned up.
➕ Avoids relying on residual list state after destruction.
➖ Introduces deferred destruction and broader ownership changes.
➖ Carries substantially greater compatibility and lifecycle risk.

Recommendation: Use the PR's narrowly scoped guard for retail-compatible Zero Hour builds. Eager link cleanup would be structurally safer, but the reverted ObjectID strategy demonstrates that changing containment state can break replay compatibility; the explicit list drain and early empty check preserve existing simulation behavior with less disruption.

Files changed (6) +41 / -163

Bug fix (3) +37 / -119

Object.cppRevert container validity tracking in Object lifecycle +9/-65

Revert container validity tracking in Object lifecycle
• Removes runtime container-ID bookkeeping and restores direct containment cleanup during destruction. Save/load processing now uses the explicitly transfer-only container ID.
Generals/Code/GameEngine/Source/GameLogic/Object/Object.cpp

OpenContain.cppGuard removal through a destructed contain list +18/-0

Guard removal through a destructed contain list
• Explicitly drains the containment list during retail-compatible destruction so its empty state remains observable. Removal now returns before calling std::find when a dangling module points to the already-emptied list.
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp

Object.cppRestore compatible Object containment lifecycle +10/-54

Restore compatible Object containment lifecycle
• Reverts pointer-validity tracking through a mirrored ObjectID and restores direct containment removal in onDestroy. Serialization retains a dedicated ID solely for save/load pointer reconstruction.
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Object.cpp

Refactor (3) +4 / -44

Object.hRestore pointer-only containment state +2/-6

Restore pointer-only containment state
• Inlines the containment pointer setter and removes the runtime container-ID synchronization API. Renames the serialized ID field to clarify that it is used only for transfer and load reconstruction.
Generals/Code/GameEngine/Include/GameLogic/Object.h

Object.hSeparate serialized containment ID from runtime state +1/-5

Separate serialized containment ID from runtime state
• Removes the retail-only container-ID setter and renames the field to indicate its exclusive use by transfer serialization.
GeneralsMD/Code/GameEngine/Include/GameLogic/Object.h

HelixContain.cppRevert Helix container-ID synchronization +1/-33

Revert Helix container-ID synchronization
• Removes retail-compatible ID maintenance for portable structures and restores the original pointer-oriented containment behavior. Non-retail builds continue clearing the containment pointer on removal.
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/HelixContain.cpp

qodo-free-for-open-source-projects · 2026-08-18T03:25:30Z

Code Review by Qodo

🐞 Bugs (3) 📘 Rule violations (0) 📎 Requirement gaps (1) 📜 Skill insights (0)

1. onDestroy() dereferences dangling m_containedBy 📎 Requirement gap ⛨ Security

Description

Object::onDestroy() dereferences m_containedBy (via m_containedBy->getContain() and then
removeFromContain()) without any safe-liveness validation, even though contained objects can
retain a dangling m_containedBy after their container is destroyed (e.g., the Troop Crawler
reinforcement-pad destruction scenario). Because the use-after-free can occur before any
OpenContain-level guard executes, the original headless-mode crash path remains and the ticket’s
requirement to remove the underlying lifetime/ownership issue is not met.

Code

GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Object.cpp[R771-774]

+	if( m_containedBy && m_containedBy->getContain() )
  {
-#if RETAIL_COMPATIBLE_CRC
-		if (m_containedByID == INVALID_ID)
-		{
-			// TheSuperHackers @bugfix Caball009 25/05/2026 Due to a potential use-after-free bug that cannot be fixed
-			// with retail compatibility, the 'contained by' pointer of this object may point to an already destroyed object.
-			// Avoid removing this object from the contain list, because it could crash the game,
-			// as the begin / end iterator for STLPort and MSVC std::list implementations depends on dynamically allocated memory.
-			DEBUG_CRASH(("container object must be valid; this looks like use-after-free"));
-		}
-		else
-		{
-			DEBUG_ASSERTCRASH(TheGameLogic->findObjectByID(m_containedByID) == m_containedBy,
-				("contained by pointer is out of sync with contained by ID"));
-
-			if (ContainModuleInterface* contain = m_containedBy->getContain())
-			{
-				contain->removeFromContain(this);
-			}
-		}
-#else
-		if (ContainModuleInterface* contain = m_containedBy->getContain())
-		{
-			contain->removeFromContain(this);
-		}
-#endif
+		m_containedBy->getContain()->removeFromContain( this );
  }

Evidence
The compliance requirement (PR Compliance ID 1 / issue #2467) is to eliminate the use-after-free in
the Troop Crawler reinforcement-pad destruction path by removing the underlying lifetime/ownership
problem. However, the updated Object::onDestroy() unconditionally dereferences m_containedBy (so
long as it is non-null) to call m_containedBy->getContain() and proceed to containment cleanup;
the codebase explicitly documents that under RETAIL_COMPATIBLE_CRC the contained-by pointer may
already be dangling, making this dereference itself a potential UAF and crash. Since the crash can
happen at the m_containedBy->getContain() dereference, any added early-return/guard inside
OpenContain::removeFromContain() cannot prevent the UAF because execution may never reach that
point.
Fix use-after-free crash when a reinforced Troop Crawler is destroyed (headless mode)
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Object.cpp[767-774]
Generals/Code/GameEngine/Source/GameLogic/Object/Object.cpp[680-687]
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Object.cpp[767-775]
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp[413-447]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`Object::onDestroy()` dereferences `m_containedBy` (`m_containedBy->getContain()` and subsequent containment cleanup) even though `m_containedBy` can be a dangling pointer when the container was already destroyed (notably in the Troop Crawler reinforcement-pad destruction path, including headless replay runs). This preserves a use-after-free crash risk and fails the compliance requirement to remove the underlying lifetime/ownership issue rather than merely adding late guards.
## Issue Context
- The compliance requirement (PR Compliance ID 1 / issue #2467) expects the fix to fully eliminate the UAF in the Troop Crawler reinforcement-pad destruction scenario.
- The codebase explicitly documents that, under `RETAIL_COMPATIBLE_CRC`, `getContainedBy()` may point to an already-destroyed object, so `m_containedBy` cannot be safely dereferenced just because it is non-null.
- Any guard/early-return inside `OpenContain::removeFromContain()` is insufficient because the UAF can occur earlier at `m_containedBy->getContain()` within `Object::onDestroy()`.
## Fix Focus Areas
- GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Object.cpp[767-775]
- Generals/Code/GameEngine/Source/GameLogic/Object/Object.cpp[680-687]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. Portable pointer not cleared 🐞 Bug ≡ Correctness

Description

HelixContain::removeFromContain() no longer clears the portable structure's faked contained-by
pointer in RETAIL_COMPATIBLE_CRC builds, leaving a stale container pointer after removal. This can
corrupt containment state and later feed the Object::onDestroy() containment cleanup path with a
dangling pointer.

Code

GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/HelixContain.cpp[R294-296]
+#if !RETAIL_COMPATIBLE_CRC
     portable->friend_setContainedBy(nullptr);
#endif

Evidence
HelixContain explicitly sets the portable structure's contained-by pointer when adding it, but the
retail-compatible removal path no longer clears it and does not invoke the normal removal logic, so
the portable structure can remain pointing at the helix after removal.
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/HelixContain.cpp[245-260]
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/HelixContain.cpp[286-305]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
For the portable-structure special case, `HelixContain` intentionally calls `portable->friend_setContainedBy(getObject())` to fake containment, but on removal it now only clears that pointer in non-retail-compatible builds. In `RETAIL_COMPATIBLE_CRC`, the portable structure keeps a stale `m_containedBy`.
## Issue Context
This code path bypasses `TransportContain::removeFromContain()` and therefore does not call `Object::onRemovedFrom()`, so there is no other cleanup of `m_containedBy` for portable structures.
## Fix Focus Areas
- Always clear the portable structure's contained-by pointer in this removal path (or call the proper removal/onRemovedFrom flow) while preserving any CRC-compat constraints through safer means (e.g., serialize IDs for CRC instead of relying on stale pointers).
### Code pointers
- GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/HelixContain.cpp[245-305]
- GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Object.cpp[767-775]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

3. Destroyed-container assertion removed 🐞 Bug ◔ Observability ⭐ New

Description

Generals Object::onContainedBy() now records a destroyed container without emitting the debug
assertion that previously identified this invalid containment state. This hides the stale-pointer
condition at its source and allows debugging builds to encounter less-local containment failures
later.

Code

Generals/Code/GameEngine/Source/GameLogic/Object/Object.cpp[L637-638]
-	DEBUG_ASSERTCRASH(containedBy == nullptr || !containedBy->isDestroyed(),
-		("Object::onContainedBy - Adding into a destroyed container"));

Evidence
The Generals implementation now assigns m_containedBy and returns without validating its lifetime,
while the sibling GeneralsMD implementation explicitly checks !containedBy->isDestroyed().
DEBUG_ASSERTCRASH invokes DebugCrash when its condition fails in diagnostic builds, proving that
deleting the assertion removes concrete detection of this invalid state.
Generals/Code/GameEngine/Source/GameLogic/Object/Object.cpp[615-624]
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Object.cpp[684-695]
Core/GameEngine/Include/Common/Debug.h[186-206]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`Object::onContainedBy()` in Generals lost its assertion against adding an object to an already destroyed container, removing an important diagnostic for the stale-pointer state involved in containment lifetime failures.

## Issue Context
The corresponding GeneralsMD implementation retains this check. Restore an unconditional `DEBUG_ASSERTCRASH` in the Generals implementation after assigning the containment state, without reintroducing the removed ID bookkeeping.

## Fix Focus Areas
- Generals/Code/GameEngine/Source/GameLogic/Object/Object.cpp[615-624]
- GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Object.cpp[684-695]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

4. CRC contains stale containedBy 🐞 Bug ≡ Correctness

Description

Object::xfer() refreshes m_xferContainedByID only for XFER_SAVE, but Object::xfer is also invoked
under XFER_CRC, so the CRC snapshot can serialize a stale contained-by ID after normal containment
changes. This weakens desync detection and can cause replay/network CRC comparisons to be
inconsistent with the actual containment state.

Code

GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Object.cpp[R4262-4265]

  	if( xfer->getXferMode() == XFER_SAVE )
  	{
  		if( m_containedBy != nullptr )
-				m_containedByID = m_containedBy->getID();
+				m_xferContainedByID = m_containedBy->getID();

Evidence
GameLogic CRC generation uses XFER_CRC and snapshots every object via xferSnapshot; Object::xfer
always xferObjectID’s m_xferContainedByID but only recomputes it for XFER_SAVE, and containment
callbacks do not update it, so CRC can serialize a value that is stale relative to m_containedBy.
Core/GameEngine/Include/Common/Xfer.h[60-69]
GeneralsMD/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp[4141-4195]
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Object.cpp[684-712]
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Object.cpp[4252-4272]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`m_xferContainedByID` is only recomputed when `xfer->getXferMode() == XFER_SAVE`, but the same field is still xfer'd during `XFER_CRC`. Since containment changes (`onContainedBy`/`onRemovedFrom`) do not update `m_xferContainedByID`, CRC snapshots can use stale IDs.
## Issue Context
`GameLogic::getCRC()` creates an `XferCRC` and calls `xferSnapshot(obj)` for every object, which drives `Object::xfer()` in `XFER_CRC` mode.
## Fix Focus Areas
- Recompute the contained-by ObjectID for both `XFER_SAVE` and `XFER_CRC` (and any other non-load modes), e.g.:
- use a local `ObjectID containedByID = m_containedBy ? m_containedBy->getID() : INVALID_ID; xfer->xferObjectID(&containedByID);`
- or update `m_xferContainedByID` whenever `m_containedBy` changes.
### Code pointers
- GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Object.cpp[4252-4272]
- GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Object.cpp[684-712]
- GeneralsMD/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp[4141-4195]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Context sources

✅ Tickets: 🎫 Destroyed Troop Crawler from reinforcement pad may crash the game due to use-after-free bug

Tip of the day

💡 Did you know, you can keep summaries lean with Finding overflow, which tucks the rest behind 'View more'

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Previous reviews

Review updated until commit 831ff73 ⚖️ Balanced

Results up to commit 7668805

🐞 Bugs (2) 📘 Rule violations (0) 📎 Requirement gaps (1) 📜 Skill insights (0)

1. onDestroy() dereferences dangling m_containedBy 📎 Requirement gap ⛨ Security

Description

Object::onDestroy() dereferences m_containedBy (via m_containedBy->getContain() and then
removeFromContain()) without any safe-liveness validation, even though contained objects can
retain a dangling m_containedBy after their container is destroyed (e.g., the Troop Crawler
reinforcement-pad destruction scenario). Because the use-after-free can occur before any
OpenContain-level guard executes, the original headless-mode crash path remains and the ticket’s
requirement to remove the underlying lifetime/ownership issue is not met.

Code

GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Object.cpp[R771-774]

+	if( m_containedBy && m_containedBy->getContain() )
	{
-#if RETAIL_COMPATIBLE_CRC
-		if (m_containedByID == INVALID_ID)
-		{
-			// TheSuperHackers @bugfix Caball009 25/05/2026 Due to a potential use-after-free bug that cannot be fixed
-			// with retail compatibility, the 'contained by' pointer of this object may point to an already destroyed object.
-			// Avoid removing this object from the contain list, because it could crash the game,
-			// as the begin / end iterator for STLPort and MSVC std::list implementations depends on dynamically allocated memory.
-			DEBUG_CRASH(("container object must be valid; this looks like use-after-free"));
-		}
-		else
-		{
-			DEBUG_ASSERTCRASH(TheGameLogic->findObjectByID(m_containedByID) == m_containedBy,
-				("contained by pointer is out of sync with contained by ID"));
-
-			if (ContainModuleInterface* contain = m_containedBy->getContain())
-			{
-				contain->removeFromContain(this);
-			}
-		}
-#else
-		if (ContainModuleInterface* contain = m_containedBy->getContain())
-		{
-			contain->removeFromContain(this);
-		}
-#endif
+		m_containedBy->getContain()->removeFromContain( this );
	}

Evidence
The compliance requirement (PR Compliance ID 1 / issue #2467) is to eliminate the use-after-free in
the Troop Crawler reinforcement-pad destruction path by removing the underlying lifetime/ownership
problem. However, the updated Object::onDestroy() unconditionally dereferences m_containedBy (so
long as it is non-null) to call m_containedBy->getContain() and proceed to containment cleanup;
the codebase explicitly documents that under RETAIL_COMPATIBLE_CRC the contained-by pointer may
already be dangling, making this dereference itself a potential UAF and crash. Since the crash can
happen at the m_containedBy->getContain() dereference, any added early-return/guard inside
OpenContain::removeFromContain() cannot prevent the UAF because execution may never reach that
point.
Fix use-after-free crash when a reinforced Troop Crawler is destroyed (headless mode)
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Object.cpp[767-774]
Generals/Code/GameEngine/Source/GameLogic/Object/Object.cpp[680-687]
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Object.cpp[767-775]
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp[413-447]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`Object::onDestroy()` dereferences `m_containedBy` (`m_containedBy->getContain()` and subsequent containment cleanup) even though `m_containedBy` can be a dangling pointer when the container was already destroyed (notably in the Troop Crawler reinforcement-pad destruction path, including headless replay runs). This preserves a use-after-free crash risk and fails the compliance requirement to remove the underlying lifetime/ownership issue rather than merely adding late guards.

## Issue Context
- The compliance requirement (PR Compliance ID 1 / issue #2467) expects the fix to fully eliminate the UAF in the Troop Crawler reinforcement-pad destruction scenario.
- The codebase explicitly documents that, under `RETAIL_COMPATIBLE_CRC`, `getContainedBy()` may point to an already-destroyed object, so `m_containedBy` cannot be safely dereferenced just because it is non-null.
- Any guard/early-return inside `OpenContain::removeFromContain()` is insufficient because the UAF can occur earlier at `m_containedBy->getContain()` within `Object::onDestroy()`.

## Fix Focus Areas
- GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Object.cpp[767-775]
- Generals/Code/GameEngine/Source/GameLogic/Object/Object.cpp[680-687]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. Portable pointer not cleared 🐞 Bug ≡ Correctness

Description

HelixContain::removeFromContain() no longer clears the portable structure's faked contained-by
pointer in RETAIL_COMPATIBLE_CRC builds, leaving a stale container pointer after removal. This can
corrupt containment state and later feed the Object::onDestroy() containment cleanup path with a
dangling pointer.

Code

GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/HelixContain.cpp[R294-296]
+#if !RETAIL_COMPATIBLE_CRC
      portable->friend_setContainedBy(nullptr);
#endif

Evidence
HelixContain explicitly sets the portable structure's contained-by pointer when adding it, but the
retail-compatible removal path no longer clears it and does not invoke the normal removal logic, so
the portable structure can remain pointing at the helix after removal.
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/HelixContain.cpp[245-260]
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/HelixContain.cpp[286-305]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
For the portable-structure special case, `HelixContain` intentionally calls `portable->friend_setContainedBy(getObject())` to fake containment, but on removal it now only clears that pointer in non-retail-compatible builds. In `RETAIL_COMPATIBLE_CRC`, the portable structure keeps a stale `m_containedBy`.

## Issue Context
This code path bypasses `TransportContain::removeFromContain()` and therefore does not call `Object::onRemovedFrom()`, so there is no other cleanup of `m_containedBy` for portable structures.

## Fix Focus Areas
- Always clear the portable structure's contained-by pointer in this removal path (or call the proper removal/onRemovedFrom flow) while preserving any CRC-compat constraints through safer means (e.g., serialize IDs for CRC instead of relying on stale pointers).

### Code pointers
- GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/HelixContain.cpp[245-305]
- GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Object.cpp[767-775]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

3. CRC contains stale containedBy 🐞 Bug ≡ Correctness

Description

Object::xfer() refreshes m_xferContainedByID only for XFER_SAVE, but Object::xfer is also invoked
under XFER_CRC, so the CRC snapshot can serialize a stale contained-by ID after normal containment
changes. This weakens desync detection and can cause replay/network CRC comparisons to be
inconsistent with the actual containment state.

Code

GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Object.cpp[R4262-4265]

		if( xfer->getXferMode() == XFER_SAVE )
		{
			if( m_containedBy != nullptr )
-				m_containedByID = m_containedBy->getID();
+				m_xferContainedByID = m_containedBy->getID();

Evidence
GameLogic CRC generation uses XFER_CRC and snapshots every object via xferSnapshot; Object::xfer
always xferObjectID’s m_xferContainedByID but only recomputes it for XFER_SAVE, and containment
callbacks do not update it, so CRC can serialize a value that is stale relative to m_containedBy.
Core/GameEngine/Include/Common/Xfer.h[60-69]
GeneralsMD/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp[4141-4195]
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Object.cpp[684-712]
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Object.cpp[4252-4272]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`m_xferContainedByID` is only recomputed when `xfer->getXferMode() == XFER_SAVE`, but the same field is still xfer'd during `XFER_CRC`. Since containment changes (`onContainedBy`/`onRemovedFrom`) do not update `m_xferContainedByID`, CRC snapshots can use stale IDs.

## Issue Context
`GameLogic::getCRC()` creates an `XferCRC` and calls `xferSnapshot(obj)` for every object, which drives `Object::xfer()` in `XFER_CRC` mode.

## Fix Focus Areas
- Recompute the contained-by ObjectID for both `XFER_SAVE` and `XFER_CRC` (and any other non-load modes), e.g.:
 - use a local `ObjectID containedByID = m_containedBy ? m_containedBy->getID() : INVALID_ID; xfer->xferObjectID(&containedByID);`
 - or update `m_xferContainedByID` whenever `m_containedBy` changes.

### Code pointers
- GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Object.cpp[4252-4272]
- GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Object.cpp[684-712]
- GeneralsMD/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp[4141-4195]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Context

✅ Tickets: 🎫 Destroyed Troop Crawler from reinforcement pad may crash the game due to use-after-free bug

qodo-free-for-open-source-projects · 2026-08-18T18:45:23Z

Code review by qodo was updated up to the latest commit 831ff73

Caball009 added 3 commits August 18, 2026 00:35

Revert "bugfix(contain): Restore retail compatibility after changes t…

3c9383b

…o Object::m_containedByID (TheSuperHackers#2868)" This reverts commit a20992d.

Revert "bugfix(object): Avoid crash with dangling contain module in O…

c5c0b8e

…bject::onDestroy() when Reinforcement Pad is destroyed before Troop Crawler drop (TheSuperHackers#2747)" This reverts commit df2224b.

Retained relevant changes.

43a5e69

Caball009 marked this pull request as ready for review August 18, 2026 03:19

qodo-free-for-open-source-projects Bot reviewed Aug 18, 2026

View reviewed changes

Comment thread GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Object.cpp

Comment thread GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/HelixContain.cpp

Comment thread GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Object.cpp

Caball009 marked this pull request as draft August 18, 2026 15:21

Caball009 force-pushed the Caball009/fix_use_after_free_troop_crawler branch from 7668805 to 4d14036 Compare August 18, 2026 17:31

Added early container size check to avoid crash.

831ff73

Caball009 force-pushed the Caball009/fix_use_after_free_troop_crawler branch from 4d14036 to 831ff73 Compare August 18, 2026 17:41

Caball009 marked this pull request as ready for review August 18, 2026 18:42

qodo-free-for-open-source-projects Bot reviewed Aug 18, 2026

View reviewed changes

Comment thread Generals/Code/GameEngine/Source/GameLogic/Object/Object.cpp

Navigation Menu

Uh oh!

bugfix(object): Avoid crash with dangling contain module in Object::onDestroy() when Troop Crawler is destroyed while en route to Reinforcement Pad - #3165

bugfix(object): Avoid crash with dangling contain module in Object::onDestroy() when Troop Crawler is destroyed while en route to Reinforcement Pad#3165
Caball009 wants to merge 4 commits into
TheSuperHackers:mainfrom
Caball009:Caball009/fix_use_after_free_troop_crawler

Caball009 commented Aug 18, 2026 •

edited

Loading

Uh oh!

qodo-free-for-open-source-projects Bot commented Aug 18, 2026 •

edited

Loading

Uh oh!

qodo-free-for-open-source-projects Bot commented Aug 18, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

qodo-free-for-open-source-projects Bot commented Aug 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

Caball009 commented Aug 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

TODO:

Uh oh!

qodo-free-for-open-source-projects Bot commented Aug 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary by Qodo

Uh oh!

qodo-free-for-open-source-projects Bot commented Aug 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

qodo-free-for-open-source-projects Bot commented Aug 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Caball009 commented Aug 18, 2026 •

edited

Loading

qodo-free-for-open-source-projects Bot commented Aug 18, 2026 •

edited

Loading

qodo-free-for-open-source-projects Bot commented Aug 18, 2026 •

edited

Loading