fixed a serious bug with byref values and callfunc_push*() where the heap was used incorrectly, causing passed data to be easily corrupted.

added callfunc_push_array() (am42810)

amxmodx/amxmodx.cpp

@@ -3054,6 +3054,7 @@ struct CallFunc_ParamInfo
 	unsigned char flags;											// flags
 	cell byrefAddr;													// byref address in caller plugin
 	cell size;														// byref size
+	cell *alloc;													// allocated block
 };
 
 #if !defined CALLFUNC_MAXPARAMS
@@ -3204,7 +3205,32 @@ static cell AMX_NATIVE_CALL callfunc_end(AMX *amx, cell *params)
 	Debugger *pDebugger = (Debugger *)pAmx->userdata[UD_DEBUGGER];
 
 	if (pDebugger)
+	{
 		pDebugger->BeginExec();
+	}
+
+	// first pass over byref things
+	for (int i = curParam - 1; i >= 0; i--)
+	{
+		if (gparamInfo[i].flags & CALLFUNC_FLAG_BYREF)
+		{
+			cell amx_addr, *phys_addr;
+			amx_Allot(pAmx, gparamInfo[i].size, &amx_addr, &phys_addr);
+			memcpy(phys_addr, gparamInfo[i].alloc, gparamInfo[i].size * sizeof(cell));
+			gparams[i] = amx_addr;
+			delete [] gparamInfo[i].alloc;
+			gparamInfo[i].alloc = NULL;
+		}
+	}
+
+	// second pass, link in reused byrefs
+	for (int i = curParam - 1; i >= 0; i--)
+	{
+		if (gparamInfo[i].flags & CALLFUNC_FLAG_BYREF_REUSED)
+		{
+			gparams[i] = gparams[gparams[i]];
+		}
+	}
 
 	// actual call
 	// Pawn - push parameters in reverse order
@@ -3226,7 +3252,9 @@ static cell AMX_NATIVE_CALL callfunc_end(AMX *amx, cell *params)
 	}
 
 	if (pDebugger)
+	{
 		pDebugger->EndExec();
+	}
 
 	// process byref params (not byref_reused)
 	for (int i = 0; i < curParam; ++i)
@@ -3305,20 +3333,13 @@ static cell AMX_NATIVE_CALL callfunc_push_byref(AMX *amx, cell *params)
 			g_CallFunc_ParamInfo[g_CallFunc_CurParam].flags = CALLFUNC_FLAG_BYREF_REUSED;
 			g_CallFunc_ParamInfo[g_CallFunc_CurParam].byrefAddr = params[1];
 			g_CallFunc_ParamInfo[g_CallFunc_CurParam].size = 1;
-			g_CallFunc_Params[g_CallFunc_CurParam++] = g_CallFunc_Params[i];
+			g_CallFunc_ParamInfo[g_CallFunc_CurParam].alloc = NULL;
-			// we are done
+			g_CallFunc_Params[g_CallFunc_CurParam++] = i;		/* referenced parameter */
 			return 0;
 		}
 	}
 
-	// not found; create an own copy
+	cell *phys_addr = new cell[1];
-	// allocate memory
-	cell *phys_addr;
-	cell amx_addr;
-	amx_Allot(g_CallFunc_Plugin->getAMX(), 
-		1, 				// 1 cell
-		&amx_addr, 
-		&phys_addr);
 
 	// copy the value to the allocated memory
 	cell *phys_addr2;
@@ -3329,7 +3350,59 @@ static cell AMX_NATIVE_CALL callfunc_push_byref(AMX *amx, cell *params)
 	g_CallFunc_ParamInfo[g_CallFunc_CurParam].flags = CALLFUNC_FLAG_BYREF;
 	g_CallFunc_ParamInfo[g_CallFunc_CurParam].byrefAddr = params[1];
 	g_CallFunc_ParamInfo[g_CallFunc_CurParam].size = 1;
-	g_CallFunc_Params[g_CallFunc_CurParam++] = amx_addr;
+	g_CallFunc_ParamInfo[g_CallFunc_CurParam].alloc = phys_addr;
+	g_CallFunc_Params[g_CallFunc_CurParam++] = 0;
+
+	return 0;
+}
+
+// native callfunc_push_array(array[], size)
+static cell AMX_NATIVE_CALL callfunc_push_array(AMX *amx, cell *params)
+{
+	if (!g_CallFunc_Plugin)
+	{
+		// scripter's fault
+		LogError(amx, AMX_ERR_NATIVE, "callfunc_push_xxx called without callfunc_begin");
+		return 0;
+	}
+
+	if (g_CallFunc_CurParam == CALLFUNC_MAXPARAMS)
+	{
+		LogError(amx, AMX_ERR_NATIVE, "callfunc_push_xxx: maximal parameters num: %d", CALLFUNC_MAXPARAMS);
+		return 0;
+	}
+
+	// search for the address; if it is found, dont create a new copy
+	for (int i = 0; i < g_CallFunc_CurParam; ++i)
+	{
+		if ((g_CallFunc_ParamInfo[i].flags & CALLFUNC_FLAG_BYREF) && (g_CallFunc_ParamInfo[i].byrefAddr == params[1]))
+		{
+			// the byrefAddr and size params should not be used; set them anyways...
+			g_CallFunc_ParamInfo[g_CallFunc_CurParam].flags = CALLFUNC_FLAG_BYREF_REUSED;
+			g_CallFunc_ParamInfo[g_CallFunc_CurParam].byrefAddr = params[1];
+			g_CallFunc_ParamInfo[g_CallFunc_CurParam].size = 1;
+			g_CallFunc_ParamInfo[g_CallFunc_CurParam].alloc = NULL;
+			g_CallFunc_Params[g_CallFunc_CurParam++] = i;		/* referenced parameter */
+			return 0;
+		}
+	}
+
+	// not found; create an own copy
+	// get the string and its length
+	cell *pArray = get_amxaddr(amx, params[1]);
+	cell array_size = params[2];
+
+	// allocate enough memory for the array
+	cell *phys_addr = new cell[array_size];
+
+	memcpy(phys_addr, pArray, array_size * sizeof(cell));
+
+	// push the address and set the reference flag so that memory is released after function call.
+	g_CallFunc_ParamInfo[g_CallFunc_CurParam].flags = CALLFUNC_FLAG_BYREF;
+	g_CallFunc_ParamInfo[g_CallFunc_CurParam].byrefAddr = params[1];
+	g_CallFunc_ParamInfo[g_CallFunc_CurParam].size = array_size;
+	g_CallFunc_ParamInfo[g_CallFunc_CurParam].alloc = phys_addr;
+	g_CallFunc_Params[g_CallFunc_CurParam++] = 0;
 
 	return 0;
 }
@@ -3359,7 +3432,8 @@ static cell AMX_NATIVE_CALL callfunc_push_str(AMX *amx, cell *params)
 			g_CallFunc_ParamInfo[g_CallFunc_CurParam].flags = CALLFUNC_FLAG_BYREF_REUSED;
 			g_CallFunc_ParamInfo[g_CallFunc_CurParam].byrefAddr = params[1];
 			g_CallFunc_ParamInfo[g_CallFunc_CurParam].size = 1;
-			g_CallFunc_Params[g_CallFunc_CurParam++] = g_CallFunc_Params[i];
+			g_CallFunc_ParamInfo[g_CallFunc_CurParam].alloc = NULL;
+			g_CallFunc_Params[g_CallFunc_CurParam++] = i;
 			// we are done
 			return 0;
 		}
@@ -3371,12 +3445,7 @@ static cell AMX_NATIVE_CALL callfunc_push_str(AMX *amx, cell *params)
 	char *str = get_amxstring(amx, params[1], 0, len);
 	
 	// allocate enough memory for the string
-	cell *phys_addr;
+	cell *phys_addr = new cell[len+1];
-	cell amx_addr;
-	amx_Allot(g_CallFunc_Plugin->getAMX(),
-		len + 1, 			// length + terminator
-		&amx_addr, 
-		&phys_addr);
 
 	// copy it to the allocated memory
 	// we assume it's unpacked
@@ -3387,7 +3456,8 @@ static cell AMX_NATIVE_CALL callfunc_push_str(AMX *amx, cell *params)
 	g_CallFunc_ParamInfo[g_CallFunc_CurParam].flags = CALLFUNC_FLAG_BYREF;
 	g_CallFunc_ParamInfo[g_CallFunc_CurParam].byrefAddr = params[1];
 	g_CallFunc_ParamInfo[g_CallFunc_CurParam].size = len + 1;
-	g_CallFunc_Params[g_CallFunc_CurParam++] = amx_addr;
+	g_CallFunc_ParamInfo[g_CallFunc_CurParam].alloc = phys_addr;
+	g_CallFunc_Params[g_CallFunc_CurParam++] = 0;
 
 	return 0;
 }
@@ -3997,6 +4067,7 @@ AMX_NATIVE_INFO amxmodx_Natives[] =
 	{"callfunc_push_intrf",		callfunc_push_byref},
 	{"callfunc_push_floatrf",	callfunc_push_byref},
 	{"callfunc_push_str",		callfunc_push_str},
+	{"callfunc_push_array",		callfunc_push_array},
 	{"change_task",				change_task},
 	{"client_cmd",				client_cmd},
 	{"client_print",			client_print},

plugins/include/amxmodx.inc

@@ -647,6 +647,7 @@ native callfunc_push_str(const VALUE[]);
 native callfunc_push_float(Float: value);
 native callfunc_push_intrf(&value);
 native callfunc_push_floatrf(& Float: value);
+native callfunc_push_array(const VALUE[], array_size);
 
 /* Make the actual call.
 * Return value of the function called. */

plugins/testsuite/callfunc_test.sma

@@ -0,0 +1,56 @@
+#include <amxmodx>
+
+public plugin_init()
+{
+	register_plugin("callfunc test", "1.0", "BAILOPAN")
+	
+	register_srvcmd("test_callfunc", "Command_Callfunc")
+}
+
+public OnCallfuncReceived(num, str[], &val, array[], array2[], size)
+{
+	server_print("num = %d (expected: %d)", num, 5)
+	server_print("str[] = ^"%s^" (expected: %s)", str, "Gaben")
+
+	server_print("val = %d (expected %d, setting to %d)", val, 62, 15)
+	val = 15
+	server_print("printing %d elements of array[] (expected: %d)", size, 6)
+	for (new i=0; i<size; i++)
+	{
+		server_print("array[%d] = %d (expected: %d)", i, array[i], i)
+	}
+	for (new i=0; i<size; i++)
+	{
+		server_print("array2[%d] = %d (expected: %d)", i, array[i], i)
+	}
+	array[0] = 5
+	array2[1] = 6
+}
+
+public Command_Callfunc()
+{
+	new a = 62
+	new hello[] = {0,1,2,3,4,5}
+	new pm = 6
+	new err
+	
+	if ((err=callfunc_begin("OnCallfuncReceived")) < 1)
+	{
+		server_print("Failed to call callfunc_begin()! Error: %d", err)
+		
+		return PLUGIN_HANDLED
+	}
+	callfunc_push_int(5)
+	callfunc_push_str("Gaben")
+	callfunc_push_intrf(a)
+	callfunc_push_array(hello, pm)
+	callfunc_push_array(hello, pm)
+	callfunc_push_int(pm)
+	callfunc_end()
+	
+	server_print("a = %d (expected: %d)", a, 15)
+	server_print("hello[0] = %d (expected: %d)", hello[0], 5)
+	server_print("hello[1] = %d (expected: %d)", hello[1], 6)
+	
+	return PLUGIN_HANDLED
+}