UNLOZE/sm-ext-dhooks2
10
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Fix changing of charptr and vectorptr returns and parameters

Browse Source 
The new buffer/vector was freed before the old function was called with the new parameters
or the return value could be used by the called. This caused undefined behavior which seemed
to be fine before, where free didn't change the user-payload. free does change the user data now,
causing the changed values to be garbage.

Wait until the next frame before deleting the newly allocated buffers/vectors, so the original code had a chance
to use the live pointers. AddFrameAction might be a bad choice if our hook happens before the game ticks, but
we can tackle that problem when it happens ;)
This commit is contained in:
Peace-Maker 2019-08-03 18:26:53 +02:00
parent 891fa5352e
commit 88451023fb
2 changed files with 19 additions and 39 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
natives.cpp
View File

@ -753,6 +753,12 @@ cell_t Native_GetParamVector(IPluginContext *pContext, const cell_t *params)
return pContext->ThrowNativeError("Invalid param type to get. Param is not a vector.");
}
static void FreeChangedVector(void *pData)
{
delete (SDKVector *)pData;
}
// native DHookSetParamVector(Handle:hParams, num, Float:vec[3])
cell_t Native_SetParamVector(IPluginContext *pContext, const cell_t *params)
{
@ -777,14 +783,13 @@ cell_t Native_SetParamVector(IPluginContext *pContext, const cell_t *params)
{
case HookParamType_VectorPtr:
{
if(paramStruct->isChanged[index])
delete *(SDKVector **)addr;
cell_t *buffer;
pContext->LocalToPhysAddr(params[3], &buffer);
*(SDKVector **)addr = new SDKVector(sp_ctof(buffer[0]), sp_ctof(buffer[1]), sp_ctof(buffer[2]));
paramStruct->isChanged[index] = true;
// Free it later (cheaply) after the function returned.
smutils->AddFrameAction(FreeChangedVector, *(SDKVector **)addr);
return 1;
}
}
@ -850,6 +855,11 @@ cell_t Native_GetReturnString(IPluginContext *pContext, const cell_t *params)
}
}
static void FreeChangedCharPtr(void *pData)
{
delete[](char *)pData;
}
//native DHookSetReturnString(Handle:hReturn, String:value[])
cell_t Native_SetReturnString(IPluginContext *pContext, const cell_t *params)
{
@ -867,13 +877,11 @@ cell_t Native_SetReturnString(IPluginContext *pContext, const cell_t *params)
{
case ReturnType_CharPtr:
{
if(returnStruct->isChanged == true)
{
delete (char *)returnStruct->newResult;
}
returnStruct->newResult = new char[strlen(value) + 1];
strcpy((char *)returnStruct->newResult, value);
returnStruct->isChanged = true;
// Free it later (cheaply) after the function returned.
smutils->AddFrameAction(FreeChangedCharPtr, returnStruct->newResult);
return 1;
}
default:
@ -905,12 +913,11 @@ cell_t Native_SetParamString(IPluginContext *pContext, const cell_t *params)
if(paramStruct->dg->params.at(index).type == HookParamType_CharPtr)
{
if(paramStruct->isChanged[index])
delete *(char **)addr;
*(char **)addr = new char[strlen(value)+1];
strcpy(*(char **)addr, value);
paramStruct->isChanged[index] = true;
// Free it later (cheaply) after the function returned.
smutils->AddFrameAction(FreeChangedCharPtr, *(char **)addr);
}
return 1;
}
@ -1299,6 +1306,8 @@ cell_t Native_SetReturnVector(IPluginContext *pContext, const cell_t *params)
{
returnStruct->newResult = new SDKVector(sp_ctof(buffer[0]), sp_ctof(buffer[1]), sp_ctof(buffer[2]));
returnStruct->isChanged = true;
// Free it later (cheaply) after the function returned.
smutils->AddFrameAction(FreeChangedVector, returnStruct->newResult);
return 1;
}

29
vhook.cpp
View File

@ -219,18 +219,6 @@ HookReturnStruct::~HookReturnStruct()
free(this->newResult);
free(this->orgResult);
}
else if (this->isChanged)
{
if (this->type == ReturnType_CharPtr)
{
delete[](char *)this->newResult;
}
else if (this->type == ReturnType_VectorPtr)
{
delete (SDKVector *)this->newResult;
}
}
}
HookParamsStruct::~HookParamsStruct()
@ -245,23 +233,6 @@ HookParamsStruct::~HookParamsStruct()
}
if (this->newParams != NULL)
{
for (int i = dg->params.size() - 1; i >= 0; i--)
{
size_t offset = GetParamOffset(this, i);
void *addr = (void **)((intptr_t)this->newParams + offset);
if (*(void **)addr == NULL)
continue;
if (dg->params.at(i).type == HookParamType_VectorPtr)
{
delete *(SDKVector **)addr;
}
else if (dg->params.at(i).type == HookParamType_CharPtr)
{
delete *(char **)addr;
}
}
free(this->newParams);
}
}