diff --git a/Generals/Code/GameEngine/Include/GameLogic/Object.h b/Generals/Code/GameEngine/Include/GameLogic/Object.h index 25c9590f8a6..dc17b8eae67 100644 --- a/Generals/Code/GameEngine/Include/GameLogic/Object.h +++ b/Generals/Code/GameEngine/Include/GameLogic/Object.h @@ -717,7 +717,7 @@ class Object : public Thing, public Snapshot Object* m_containedBy; /**< an object can only be contained by at most one other object, this is that object (if present) */ - ObjectID m_xferContainedByID; ///< xfer uses IDs to store pointers and looks them up after + ObjectID m_containedByID; ///< ID of the object we're contained by; only to be used when m_containedBy cannot be used UnsignedInt m_containedByFrame; ///< frame we were contained by m_containedBy Real m_constructionPercent; ///< for objects being built ... this is the amount completed (0.0 to 100.0) diff --git a/Generals/Code/GameEngine/Source/GameLogic/Object/Object.cpp b/Generals/Code/GameEngine/Source/GameLogic/Object/Object.cpp index 85bd61efa71..37e33d4a96e 100644 --- a/Generals/Code/GameEngine/Source/GameLogic/Object/Object.cpp +++ b/Generals/Code/GameEngine/Source/GameLogic/Object/Object.cpp @@ -173,7 +173,7 @@ Object::Object( const ThingTemplate *tt, const ObjectStatusMaskType &objectStatu m_physics(nullptr), m_geometryInfo(tt->getTemplateGeometryInfo()), m_containedBy(nullptr), - m_xferContainedByID(INVALID_ID), + m_containedByID(INVALID_ID), m_containedByFrame(0), m_behaviors(nullptr), m_body(nullptr), @@ -621,6 +621,22 @@ void Object::onContainedBy( Object *containedBy ) clearStatus( MAKE_OBJECT_STATUS_MASK( OBJECT_STATUS_MASKED ) ); m_containedBy = containedBy; m_containedByFrame = TheGameLogic->getFrame(); + +#if RETAIL_COMPATIBLE_CRC + // TheSuperHackers @info Set INVALID_ID if the container object was destroyed + // to indicate that the pointer will become a dangling pointer in the next frame. + if (containedBy && !containedBy->isDestroyed()) + { + m_containedByID = containedBy->getID(); + } + else + { + m_containedByID = INVALID_ID; + } +#else + DEBUG_ASSERTCRASH(containedBy == nullptr || !containedBy->isDestroyed(), + ("Object::onContainedBy - Adding into a destroyed container")); +#endif } //------------------------------------------------------------------------------------------------- @@ -631,6 +647,10 @@ void Object::onRemovedFrom( Object *removedFrom ) clearStatus( MAKE_OBJECT_STATUS_MASK2( OBJECT_STATUS_MASKED, OBJECT_STATUS_UNSELECTABLE ) ); m_containedBy = nullptr; m_containedByFrame = 0; + +#if RETAIL_COMPATIBLE_CRC + m_containedByID = INVALID_ID; +#endif } //------------------------------------------------------------------------------------------------- @@ -681,9 +701,33 @@ void Object::onDestroy() { // This is the old cleanUpContain safeguard. Say goodbye so they don't try to look us up. - if( m_containedBy && m_containedBy->getContain() ) + if (m_containedBy) { - m_containedBy->getContain()->removeFromContain( this ); +#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 } // @@ -3727,16 +3771,18 @@ void Object::xfer( Xfer *xfer ) // No, the contain module is just going to friend_ reach in and set this for us. // Containers more complicated than Open (like Tunnel) can't do that. Our variable, // our responsibility. +#if !RETAIL_COMPATIBLE_CRC + // TheSuperHackers @tweak Contained by ID is already set with retail compatibility; don't overwrite it. if( xfer->getXferMode() == XFER_SAVE ) { if( m_containedBy != nullptr ) - m_xferContainedByID = m_containedBy->getID(); + m_containedByID = m_containedBy->getID(); else - m_xferContainedByID = INVALID_ID; + m_containedByID = INVALID_ID; } +#endif - - xfer->xferObjectID( &m_xferContainedByID ); + xfer->xferObjectID( &m_containedByID ); } // contained by frame @@ -3961,8 +4007,8 @@ void Object::xfer( Xfer *xfer ) //------------------------------------------------------------------------------------------------- void Object::loadPostProcess() { - if( m_xferContainedByID != INVALID_ID ) - m_containedBy = TheGameLogic->findObjectByID(m_xferContainedByID); + if( m_containedByID != INVALID_ID ) + m_containedBy = TheGameLogic->findObjectByID(m_containedByID); else m_containedBy = nullptr; diff --git a/GeneralsMD/Code/GameEngine/Include/GameLogic/Object.h b/GeneralsMD/Code/GameEngine/Include/GameLogic/Object.h index bfdf333393f..29d391b81a3 100644 --- a/GeneralsMD/Code/GameEngine/Include/GameLogic/Object.h +++ b/GeneralsMD/Code/GameEngine/Include/GameLogic/Object.h @@ -761,7 +761,7 @@ class Object : public Thing, public Snapshot Object* m_containedBy; /**< an object can only be contained by at most one other object, this is that object (if present) */ - ObjectID m_xferContainedByID; ///< xfer uses IDs to store pointers and looks them up after + ObjectID m_containedByID; ///< ID of the object we're contained by; only to be used when m_containedBy cannot be used UnsignedInt m_containedByFrame; ///< frame we were contained by m_containedBy Real m_constructionPercent; ///< for objects being built ... this is the amount completed (0.0 to 100.0) diff --git a/GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Object.cpp b/GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Object.cpp index a8029b4ec8d..377a82f8832 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Object.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Object.cpp @@ -182,7 +182,7 @@ Object::Object( const ThingTemplate *tt, const ObjectStatusMaskType &objectStatu m_physics(nullptr), m_geometryInfo(tt->getTemplateGeometryInfo()), m_containedBy(nullptr), - m_xferContainedByID(INVALID_ID), + m_containedByID(INVALID_ID), m_containedByFrame(0), m_behaviors(nullptr), m_body(nullptr), @@ -691,6 +691,22 @@ void Object::onContainedBy( Object *containedBy ) m_containedBy = containedBy; m_containedByFrame = TheGameLogic->getFrame(); +#if RETAIL_COMPATIBLE_CRC + // TheSuperHackers @info Set INVALID_ID if the container object was destroyed + // to indicate that the pointer will become a dangling pointer in the next frame. + if (containedBy && !containedBy->isDestroyed()) + { + m_containedByID = containedBy->getID(); + } + else + { + m_containedByID = INVALID_ID; + } +#else + DEBUG_ASSERTCRASH(containedBy == nullptr || !containedBy->isDestroyed(), + ("Object::onContainedBy - Adding into a destroyed container")); +#endif + handlePartitionCellMaintenance(); // which should unlook me now that I am contained } @@ -704,6 +720,10 @@ void Object::onRemovedFrom( Object *removedFrom ) m_containedBy = nullptr; m_containedByFrame = 0; +#if RETAIL_COMPATIBLE_CRC + m_containedByID = INVALID_ID; +#endif + handlePartitionCellMaintenance(); // get a clean look, now that I am outdoors, again } @@ -756,9 +776,33 @@ void Object::onDestroy() { // This is the old cleanUpContain safeguard. Say goodbye so they don't try to look us up. - if( m_containedBy && m_containedBy->getContain() ) + if (m_containedBy) { - m_containedBy->getContain()->removeFromContain( this ); +#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 } // @@ -4246,16 +4290,18 @@ void Object::xfer( Xfer *xfer ) // No, the contain module is just going to friend_ reach in and set this for us. // Containers more complicated than Open (like Tunnel) can't do that. Our variable, // our responsibility. +#if !RETAIL_COMPATIBLE_CRC + // TheSuperHackers @tweak Contained by ID is already set with retail compatibility; don't overwrite it. if( xfer->getXferMode() == XFER_SAVE ) { if( m_containedBy != nullptr ) - m_xferContainedByID = m_containedBy->getID(); + m_containedByID = m_containedBy->getID(); else - m_xferContainedByID = INVALID_ID; + m_containedByID = INVALID_ID; } +#endif - - xfer->xferObjectID( &m_xferContainedByID ); + xfer->xferObjectID( &m_containedByID ); } // contained by frame @@ -4480,8 +4526,8 @@ void Object::xfer( Xfer *xfer ) //------------------------------------------------------------------------------------------------- void Object::loadPostProcess() { - if( m_xferContainedByID != INVALID_ID ) - m_containedBy = TheGameLogic->findObjectByID(m_xferContainedByID); + if( m_containedByID != INVALID_ID ) + m_containedBy = TheGameLogic->findObjectByID(m_containedByID); else m_containedBy = nullptr;