diff --git a/Generals/Code/GameEngine/Include/GameLogic/Object.h b/Generals/Code/GameEngine/Include/GameLogic/Object.h index 7a1a25260a9..a2dcdd36606 100644 --- a/Generals/Code/GameEngine/Include/GameLogic/Object.h +++ b/Generals/Code/GameEngine/Include/GameLogic/Object.h @@ -424,14 +424,10 @@ class Object : public Thing, public Snapshot void onContainedBy( Object *containedBy ); void onRemovedFrom( Object *removedFrom ); Int getTransportSlotCount() const; - void friend_setContainedBy( Object *containedBy ); + void friend_setContainedBy( Object *containedBy ) { m_containedBy = containedBy; } const Object* getEnclosingContainedBy() const; ///< Find the first enclosing container in the containment chain. const Object* getOuterObject() const; ///< Get the top-level object -#if RTS_ZEROHOUR && RETAIL_COMPATIBLE_CRC - void friend_setContainedByID(ObjectID id) { m_containedByID = id; } -#endif - // Special Powers ------------------------------------------------------------------------------- SpecialPowerModuleInterface *getSpecialPowerModule( const SpecialPowerTemplate *specialPowerTemplate ) const; void doSpecialPower( const SpecialPowerTemplate *specialPowerTemplate, UnsignedInt commandOptions, Bool forced = false ); ///< execute power @@ -721,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_containedByID; ///< ID of the object we're contained by; only to be used when m_containedBy cannot be used + ObjectID m_xferContainedByID; ///< xfer uses IDs to store pointers and looks them up after 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 c24af6a0175..448d1225825 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_containedByID(INVALID_ID), + m_xferContainedByID(INVALID_ID), m_containedByFrame(0), m_behaviors(nullptr), m_body(nullptr), @@ -621,22 +621,6 @@ 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 } //------------------------------------------------------------------------------------------------- @@ -647,10 +631,6 @@ 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 } //------------------------------------------------------------------------------------------------- @@ -674,15 +654,6 @@ Int Object::getTransportSlotCount() const return count; } -void Object::friend_setContainedBy(Object* containedBy) -{ - m_containedBy = containedBy; - -#if !RETAIL_COMPATIBLE_CRC - m_containedByFrame = containedBy ? TheGameLogic->getFrame() : 0; -#endif -} - const Object* Object::getEnclosingContainedBy() const { for (const Object* child = this, *container = getContainedBy(); container; child = container, container = container->getContainedBy()) @@ -710,33 +681,9 @@ void Object::onDestroy() { // This is the old cleanUpContain safeguard. Say goodbye so they don't try to look us up. - if (m_containedBy) + 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 ); } // @@ -3781,19 +3728,16 @@ 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. -#else if( xfer->getXferMode() == XFER_SAVE ) { if( m_containedBy != nullptr ) - m_containedByID = m_containedBy->getID(); + m_xferContainedByID = m_containedBy->getID(); else - m_containedByID = INVALID_ID; + m_xferContainedByID = INVALID_ID; } -#endif - xfer->xferObjectID( &m_containedByID ); + + xfer->xferObjectID( &m_xferContainedByID ); } // contained by frame @@ -4018,8 +3962,8 @@ void Object::xfer( Xfer *xfer ) //------------------------------------------------------------------------------------------------- void Object::loadPostProcess() { - if( m_containedByID != INVALID_ID ) - m_containedBy = TheGameLogic->findObjectByID(m_containedByID); + if( m_xferContainedByID != INVALID_ID ) + m_containedBy = TheGameLogic->findObjectByID(m_xferContainedByID); else m_containedBy = nullptr; diff --git a/GeneralsMD/Code/GameEngine/Include/GameLogic/Object.h b/GeneralsMD/Code/GameEngine/Include/GameLogic/Object.h index 90fc52b3960..13019ab3fa7 100644 --- a/GeneralsMD/Code/GameEngine/Include/GameLogic/Object.h +++ b/GeneralsMD/Code/GameEngine/Include/GameLogic/Object.h @@ -452,10 +452,6 @@ class Object : public Thing, public Snapshot const Object* getEnclosingContainedBy() const; ///< Find the first enclosing container in the containment chain. const Object* getOuterObject() const; ///< Get the top-level object -#if RTS_ZEROHOUR && RETAIL_COMPATIBLE_CRC - void friend_setContainedByID(ObjectID id) { m_containedByID = id; } -#endif - // Special Powers ------------------------------------------------------------------------------- SpecialPowerModuleInterface *getSpecialPowerModule( const SpecialPowerTemplate *specialPowerTemplate ) const; void doSpecialPower( const SpecialPowerTemplate *specialPowerTemplate, UnsignedInt commandOptions, Bool forced = false ); ///< execute power @@ -764,7 +760,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_containedByID; ///< ID of the object we're contained by; only to be used when m_containedBy cannot be used + ObjectID m_xferContainedByID; ///< xfer uses IDs to store pointers and looks them up after 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/Contain/HelixContain.cpp b/GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/HelixContain.cpp index c900e02d348..c2c470bb3ba 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/HelixContain.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/HelixContain.cpp @@ -255,23 +255,8 @@ void HelixContain::addToContainList( Object *obj ) m_portableStructureID = portable->getID(); portable->friend_setContainedBy( getObject() );//fool portable into thinking my object is his container -#if RETAIL_COMPATIBLE_CRC - Object* containedBy = getObject(); - - // 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()) - { - portable->friend_setContainedByID(containedBy->getID()); - } - else - { - portable->friend_setContainedByID(INVALID_ID); - } -#else DEBUG_ASSERTCRASH(getObject() == nullptr || !getObject()->isDestroyed(), ("HelixContain::addToContainList - Adding to a destroyed container")); -#endif } else TransportContain::addToContainList( obj ); @@ -291,23 +276,8 @@ void HelixContain::addToContain( Object *obj ) m_portableStructureID = portable->getID(); portable->friend_setContainedBy( getObject() );//fool portable into thinking my object is his container -#if RETAIL_COMPATIBLE_CRC - Object* containedBy = getObject(); - - // 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()) - { - portable->friend_setContainedByID(containedBy->getID()); - } - else - { - portable->friend_setContainedByID(INVALID_ID); - } -#else DEBUG_ASSERTCRASH(getObject() == nullptr || !getObject()->isDestroyed(), ("HelixContain::addToContain - Adding to a destroyed container")); -#endif } else TransportContain::addToContain( obj ); @@ -321,9 +291,7 @@ void HelixContain::removeFromContain( Object *obj, Bool exposeStealthUnits ) Object *portable = getPortableStructure(); if ( portable ) { -#if RETAIL_COMPATIBLE_CRC - portable->friend_setContainedByID(INVALID_ID); -#else +#if !RETAIL_COMPATIBLE_CRC portable->friend_setContainedBy(nullptr); #endif diff --git a/GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp b/GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp index baea90bf2c5..cabff786ad2 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp @@ -177,6 +177,15 @@ OpenContain::~OpenContain() ("OpenContain %s: m_xferContainIDList is not empty but should be", getObject()->getTemplate()->getName().str() ) ); +#if RETAIL_COMPATIBLE_CRC + // TheSuperHackers @bugfix Caball009 18/08/2026 Due to a potential use-after-free bug that cannot be fixed + // with retail compatibility, it's desirable to be able to check if the contain list is empty after its destruction. + // Empty the list explicitly to reset the list size. + while (!m_containList.empty()) + { + m_containList.pop_front(); + } +#endif } //------------------------------------------------------------------------------------------------- @@ -434,6 +443,15 @@ void OpenContain::removeFromContain( Object *rider, Bool exposeStealthUnits ) } +#if RETAIL_COMPATIBLE_CRC + // TheSuperHackers @bugfix Caball009 18/08/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. + // Check the list size before executing the find operation below, otherwise the game crashes if the list + // was already destructed. + if (m_containList.empty()) + return; +#endif + ContainedItemsList::iterator it = std::find(m_containList.begin(), m_containList.end(), rider); if (it != m_containList.end()) { diff --git a/GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Object.cpp b/GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Object.cpp index 800b5c20e4a..a9a874cb13c 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_containedByID(INVALID_ID), + m_xferContainedByID(INVALID_ID), m_containedByFrame(0), m_behaviors(nullptr), m_body(nullptr), @@ -691,21 +691,8 @@ 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 + ("Object::onContainedBy - Adding to a destroyed container")); handlePartitionCellMaintenance(); // which should unlook me now that I am contained @@ -720,10 +707,6 @@ 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 } @@ -785,33 +768,9 @@ void Object::onDestroy() { // This is the old cleanUpContain safeguard. Say goodbye so they don't try to look us up. - if (m_containedBy) + 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 ); } // @@ -4300,19 +4259,16 @@ 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. -#else if( xfer->getXferMode() == XFER_SAVE ) { if( m_containedBy != nullptr ) - m_containedByID = m_containedBy->getID(); + m_xferContainedByID = m_containedBy->getID(); else - m_containedByID = INVALID_ID; + m_xferContainedByID = INVALID_ID; } -#endif - xfer->xferObjectID( &m_containedByID ); + + xfer->xferObjectID( &m_xferContainedByID ); } // contained by frame @@ -4537,8 +4493,8 @@ void Object::xfer( Xfer *xfer ) //------------------------------------------------------------------------------------------------- void Object::loadPostProcess() { - if( m_containedByID != INVALID_ID ) - m_containedBy = TheGameLogic->findObjectByID(m_containedByID); + if( m_xferContainedByID != INVALID_ID ) + m_containedBy = TheGameLogic->findObjectByID(m_xferContainedByID); else m_containedBy = nullptr;