2.3: Fix an invalid pointer dereferencing issue in the script events system.

Closes #2300.

Author: Safety0ff

Diff for: lib/script/event.c

@@ -43,7 +43,6 @@ static ACTIVE_TRIGGER	*psAddedTriggers = NULL;
 
 /** The trigger which is currently firing */
 static ACTIVE_TRIGGER	*psFiringTrigger = NULL;
-static BOOL		triggerChanged;
 static UDWORD		updateTime;
 
 /** The currently allocated contexts */
@@ -98,6 +97,15 @@ static void eventAddTrigger(ACTIVE_TRIGGER *psTrigger);
 // Free up a trigger
 static void eventFreeTrigger(ACTIVE_TRIGGER *psTrigger);
 
+// Remove triggers marked for deletion
+static void eventPruneList(ACTIVE_TRIGGER **psList);
+static void eventPruneLists(void)
+{
+	eventPruneList(&psTrigList);
+	eventPruneList(&psCallbackList);
+	eventPruneList(&psAddedTriggers);
+}
+
 //resets the event timer - updateTime
 void eventTimeReset(UDWORD initTime)
 {
@@ -834,10 +842,8 @@ static BOOL eventInitTrigger(ACTIVE_TRIGGER **ppsTrigger, SCRIPT_CONTEXT *psCont
 	TRIGGER_DATA	*psTrigData;
 	UDWORD			testTime;
 
-	ASSERT( event < psContext->psCode->numEvents,
-		"eventAddTrigger: Event out of range" );
-	ASSERT( trigger < psContext->psCode->numTriggers,
-		"eventAddTrigger: Trigger out of range" );
+	ASSERT( event < psContext->psCode->numEvents, "Event out of range" );
+	ASSERT( trigger < psContext->psCode->numTriggers, "Trigger out of range" );
 	if (trigger == -1)
 	{
 		return false;
@@ -861,6 +867,7 @@ static BOOL eventInitTrigger(ACTIVE_TRIGGER **ppsTrigger, SCRIPT_CONTEXT *psCont
 	psNewTrig->type = (SWORD)psTrigData->type;
 	psNewTrig->event = (UWORD)event;
 	psNewTrig->offset = 0;
+	psNewTrig->deactivated = false;
 
 	*ppsTrigger = psNewTrig;
 
@@ -897,6 +904,7 @@ BOOL eventLoadTrigger(UDWORD time, SCRIPT_CONTEXT *psContext,
 	psNewTrig->type = (SWORD)type;
 	psNewTrig->event = (UWORD)event;
 	psNewTrig->offset = (UWORD)offset;
+	psNewTrig->deactivated = false;
 
 	eventAddTrigger(psNewTrig);
 
@@ -947,13 +955,14 @@ BOOL eventAddPauseTrigger(SCRIPT_CONTEXT *psContext, UDWORD event, UDWORD offset
 	psNewTrig->type = TR_PAUSE;
 	psNewTrig->event = (UWORD)event;
 	psNewTrig->offset = (UWORD)offset;
+	psNewTrig->deactivated = false;
 
 	// store the new trigger
 	psNewTrig->psNext = psAddedTriggers;
 	psAddedTriggers = psNewTrig;
 
-	// tell the event system the trigger has been changed
-	triggerChanged = true;
+	// mark the trigger for deletion
+	psFiringTrigger->deactivated = true;
 
 	return true;
 }
@@ -1041,23 +1050,22 @@ void eventFireCallbackTrigger(TRIGGER_TYPE callback)
 					psPrev->psNext = psNext;
 				}
 
-				triggerChanged = false;
 				psFiringTrigger = psCurr;
-				if (!interpRunScript(psCurr->psContext, IRT_EVENT, psCurr->event, psCurr->offset)) // this could set triggerChanged
+				if (!interpRunScript(psCurr->psContext, IRT_EVENT, psCurr->event, psCurr->offset)) // this could set psCurr->deactivated
 				{
 					ASSERT(false, "eventFireCallbackTrigger: event %s: code failed",
 					       eventGetEventID(psCurr->psContext->psCode, psCurr->event));
 				}
-				if (triggerChanged)
+				if (psCurr->deactivated)
 				{
 					// don't need to add the trigger again - just free it
 					eventFreeTrigger(psCurr);
 				}
 				else
 				{
 					// make sure the trigger goes back into the system
-					psFiringTrigger->psNext = psAddedTriggers;
-					psAddedTriggers = psFiringTrigger;
+					psCurr->psNext = psAddedTriggers;
+					psAddedTriggers = psCurr;
 				}
 			}
 			else
@@ -1071,6 +1079,9 @@ void eventFireCallbackTrigger(TRIGGER_TYPE callback)
 		}
 	}
 
+	// Delete marked triggers now
+	eventPruneLists();
+
 	// Now add all the new triggers
 	for(psCurr = psAddedTriggers; psCurr; psCurr=psNext)
 	{
@@ -1089,7 +1100,7 @@ static BOOL eventFireTrigger(ACTIVE_TRIGGER *psTrigger)
 	INTERP_VAL		sResult;
 
 	fired = false;
-
+	psFiringTrigger = psTrigger;
 
 	// If this is a code trigger see if it fires
 	if (psTrigger->type == TR_CODE)
@@ -1154,15 +1165,10 @@ void eventProcessTriggers(UDWORD currTime)
 		psCurr = psTrigList;
 		psTrigList = psTrigList->psNext;
 
-		// Store the trigger so that I can tell if the event changes
-		// the trigger assigned to it
-		psFiringTrigger = psCurr;
-		triggerChanged = false;
-
 		// Run the trigger
-		if (eventFireTrigger(psCurr))	// This might set triggerChanged
+		if (eventFireTrigger(psCurr))	// This might mark the trigger for deletion
 		{
-			if (triggerChanged || psCurr->type == TR_WAIT)
+			if (psCurr->deactivated || psCurr->type == TR_WAIT)
 			{
 				// remove the trigger
 				eventFreeTrigger(psCurr);
@@ -1195,6 +1201,9 @@ void eventProcessTriggers(UDWORD currTime)
 		}
 	}
 
+	// Delete marked triggers now
+	eventPruneLists();
+
 	// Now add all the new triggers
 	for(psCurr = psAddedTriggers; psCurr; psCurr=psNext)
 	{
@@ -1205,56 +1214,56 @@ void eventProcessTriggers(UDWORD currTime)
 	psAddedTriggers = NULL;
 }
 
-
-// remove a trigger from a list
-static void eventRemoveTriggerFromList(ACTIVE_TRIGGER **ppsList,
-								SCRIPT_CONTEXT *psContext,
-								SDWORD event, SDWORD *pTrigger)
+// remove all marked triggers
+static void eventPruneList(ACTIVE_TRIGGER **ppsList)
 {
-	ACTIVE_TRIGGER	*psCurr, *psPrev=NULL;
+	ACTIVE_TRIGGER	**ppsCurr = ppsList, *psTemp;
 
-	if (((*ppsList) != NULL) &&
-		(*ppsList)->event == event &&
-		(*ppsList)->psContext == psContext)
+	while (*ppsCurr)
 	{
-		if ((*ppsList)->type == TR_PAUSE)
+		if ((*ppsCurr)->deactivated)
 		{
-			// pause trigger, don't remove it,
-			// just note the type for when the pause finishes
-			(*ppsList)->trigger = (SWORD)*pTrigger;
-			*pTrigger = -1;
+			psTemp = (*ppsCurr)->psNext;
+			free(*ppsCurr);
+			*ppsCurr = psTemp;
 		}
 		else
 		{
-			psCurr = *ppsList;
-			*ppsList = (*ppsList)->psNext;
-			free(psCurr);
+			ppsCurr = &(*ppsCurr)->psNext;
 		}
 	}
-	else
+}
+
+// Mark a trigger for removal from a list
+static void eventMarkTriggerInList(ACTIVE_TRIGGER **ppsList,
+								SCRIPT_CONTEXT *psContext,
+								SDWORD event, SDWORD *pTrigger)
+{
+	ACTIVE_TRIGGER	**ppsCurr;
+
+	for (ppsCurr = ppsList;; ppsCurr = &(*ppsCurr)->psNext)
 	{
-		for(psCurr=*ppsList; psCurr; psCurr=psCurr->psNext)
-		{
-			if (psCurr->event == event &&
-				psCurr->psContext == psContext)
-			{
-				break;
-			}
-			psPrev = psCurr;
-		}
-		if (psCurr && psCurr->type == TR_PAUSE)
+		if (!(*ppsCurr))
 		{
-			// pause trigger, don't remove it,
-			// just note the type for when the pause finishes
-			psCurr->trigger = (SWORD)*pTrigger;
-			*pTrigger = -1;
+			return;
 		}
-		else if (psCurr)
+		else if ((*ppsCurr)->event == event &&
+				 (*ppsCurr)->psContext == psContext)
 		{
-			psPrev->psNext = psCurr->psNext;
-			free(psCurr);
+			break;
 		}
 	}
+	if ((*ppsCurr)->type == TR_PAUSE)
+	{
+		// pause trigger, don't remove it,
+		// just note the type for when the pause finishes
+		(*ppsCurr)->trigger = (SWORD)*pTrigger;
+		*pTrigger = -1;
+	}
+	else
+	{
+		(*ppsCurr)->deactivated = true;
+	}
 }
 
 
@@ -1281,14 +1290,14 @@ BOOL eventSetTrigger(void)
 	psContext = psFiringTrigger->psContext;
 	if (psFiringTrigger->event == event)
 	{
-		triggerChanged = true;
+		psFiringTrigger->deactivated = true;
 	}
 	else
 	{
-		// Remove any old trigger from the list
-		eventRemoveTriggerFromList(&psTrigList, psContext, event, &trigger);
-		eventRemoveTriggerFromList(&psCallbackList, psContext, event, &trigger);
-		eventRemoveTriggerFromList(&psAddedTriggers, psContext, event, &trigger);
+		// Mark the old trigger in the lists
+		eventMarkTriggerInList(&psTrigList, psContext, event, &trigger);
+		eventMarkTriggerInList(&psCallbackList, psContext, event, &trigger);
+		eventMarkTriggerInList(&psAddedTriggers, psContext, event, &trigger);
 	}
 
 	// Create a new trigger if necessary

Diff for: lib/script/event.h

@@ -83,7 +83,7 @@ typedef struct _active_trigger
 	SWORD				trigger;
 	UWORD				event;
 	UWORD				offset;
-
+	BOOL				deactivated;	// Whether the trigger is marked for deletion
 	struct _active_trigger *psNext;
 } ACTIVE_TRIGGER;