* Possible fix to ers/status_change crashing. [FlavioJS]

- this patch is untested. Didn't find anyone willing to test it and I can't reproduce the crash so can't test the patch.
  symptom:
   status_change_entry.timer is being modified after the entry is freed.
   ers uses that memory location as a pointer when the entry is freed, it crashes when accessing the contents when it points to an invalid location.
  assumption:
   status_change_start starts an already existing status.
   as a consequence of something called inside status_change_start the status is ended.
   when the sce is finally being modified, it's modifying a freed entry.

git-svn-id: https://rathena.svn.sourceforge.net/svnroot/rathena/trunk@12058 54d463be-8e91-2dee-dedb-b68131a5f0ec

2 files changed, 31 insertions, 10 deletions

diff --git a/Changelog-Trunk.txt b/Changelog-Trunk.txt
index 82bb3d58f..77c7050bd 100644
--- a/Changelog-Trunk.txt
+++ b/Changelog-Trunk.txt
@@ -3,6 +3,18 @@ Date	Added
 AS OF SVN REV. 5091, WE ARE NOW USING TRUNK.  ALL UNTESTED BUGFIXES/FEATURES GO INTO TRUNK.
 IF YOU HAVE A WORKING AND TESTED BUGFIX PUT IT INTO STABLE AS WELL AS TRUNK.
 
+2008/01/12
+	* Possible fix to ers/status_change crashing. [FlavioJS]
+	- this patch is untested. Didn't find anyone willing to test it and I can't 
+	  reproduce the crash so can't test the patch.
+	  symptom:
+	   status_change_entry.timer is being modified after the entry is freed.
+	   ers uses that memory location as a pointer when the entry is freed, 
+	   it crashes when accessing the contents when it points to an invalid location.
+	  assumption:
+	   status_change_start starts an already existing status.
+	   as a consequence of something called inside status_change_start the status is ended.
+	   when the sce is finally being modified, it's modifying a freed entry.
 2008/01/11
 	* Implemented the extra damage bonus to TK_JUMPKICK when it is used while
 	  running (however what the bonus's equation is has been lost to time, so
diff --git a/src/map/status.c b/src/map/status.c
index 768d9fc1d..1e3fd7e3a 100644
--- a/src/map/status.c
+++ b/src/map/status.c
@@ -5000,15 +5000,18 @@ int status_change_start(struct block_list* bl,enum sc_type type,int rate,int val
 				if(sce->val1 > val1)
 					return 1; //Return true to not mess up skill animations. [Skotlex
 		}
-		(sc->count)--;
-		if (sce->timer != -1)
-			delete_timer(sce->timer, status_change_timer);
-		sce->timer = -1;
 	}
 	//NOTE: avoiding returning after this point, or if you must return a failure, use this to properly cleanup any existing data. 
-#define sc_start_abort(ret) { \
-	if (sce) { ers_free(sc_data_ers, sce); sc->data[type] = NULL; } \
-	return ret; }
+#define sc_start_abort(ret) \
+	do{ \
+		if((sce=sc->data[type])){ \
+			--(sc->count); \
+			sc->data[type] = NULL; \
+			if( sce->timer != INVALID_TIMER ) delete_timer(sce->timer, status_change_timer); \
+			ers_free(sc_data_ers, sce); \
+			return ret; \
+		} \
+	}while(0)
 
 	vd = status_get_viewdata(bl);
 	calc_flag = StatusChangeFlagTable[type];
@@ -6101,10 +6104,16 @@ int status_change_start(struct block_list* bl,enum sc_type type,int rate,int val
 	else if (sd) //Send packet to self otherwise (disguised player?)
 		clif_status_load(bl,StatusIconChangeTable[type],1);
 
-	(sc->count)++;
-
-	if (!sce) //Not null when overwriting existing sc.
+	if((sce=sc->data[type]))
+	{// reuse old sc
+		if( sce->timer != INVALID_TIMER )
+			delete_timer(sce->timer, status_change_timer);
+	}
+	else
+	{// new sc
+		++(sc->count);
 		sce = sc->data[type] = ers_alloc(sc_data_ers, struct status_change_entry);
+	}
 	sce->val1 = val1;
 	sce->val2 = val2;
 	sce->val3 = val3;