Use C4DeletionTrackable to fix use after free in C4Effect::Kill

legacyclonk · Nov 18, 2023 · 0cc4399 · 0cc4399
1 parent b9353d7
commit 0cc4399
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 2 deletions.
diff --git a/src/C4Effect.cpp b/src/C4Effect.cpp
@@ -364,21 +364,42 @@ void C4Effect::Execute(C4Object *pObj)
 
 void C4Effect::Kill(C4Object *pObj)
 {
+	const auto deletionTracker = TrackDeletion();
 	// active?
 	C4Effect *pLastRemovedEffect = nullptr;
 	if (IsActive())
+	{
 		// then temp remove all higher priority effects
 		TempRemoveUpperEffects(pObj, false, &pLastRemovedEffect);
+	}
 	else
+	{
 		// otherwise: temp reactivate before real removal
 		// this happens only if a lower priority effect removes an upper priority effect in its add- or removal-call
-		if (pFnStart && iPriority != 1) pFnStart->Exec(pCommandTarget, {C4VObj(pObj), C4VInt(iNumber), C4VInt(C4FxCall_TempAddForRemoval)}, false, true);
+		if (pFnStart && iPriority != 1)
+		{
+			pFnStart->Exec(pCommandTarget, {C4VObj(pObj), C4VInt(iNumber), C4VInt(C4FxCall_TempAddForRemoval)}, false, true);
+			if (deletionTracker.IsDeleted())
+			{
+				return;
+			}
+		}
+	}
 	// remove this effect
 	int32_t iPrevPrio = iPriority; SetDead();
 	if (pFnStop)
+	{
 		if (pFnStop->Exec(pCommandTarget, {C4VObj(pObj), C4VInt(iNumber)}, false, true).getInt() == C4Fx_Stop_Deny)
+		{
 			// effect denied to be removed: recover
 			iPriority = iPrevPrio;
+		}
+
+		if (deletionTracker.IsDeleted())
+		{
+			return;
+		}
+	}
 	// reactivate other effects
 	TempReaddUpperEffects(pObj, pLastRemovedEffect);
 }

diff --git a/src/C4Effects.h b/src/C4Effects.h
@@ -24,6 +24,7 @@
 
 #include "C4Aul.h"
 #include "C4Constants.h"
+#include "C4DeletionTrackable.h"
 #include "C4EnumeratedObjectPtr.h"
 #include "C4ValueList.h"
 
@@ -73,7 +74,7 @@ typedef unsigned long C4ID;
 #define C4Fx_FireMode_Last      3 // largest valid fire mode
 
 // generic object effect
-class C4Effect
+class C4Effect : private C4DeletionTrackable
 {
 public:
 	char Name[C4MaxDefString + 1]; // name of effect