From 46acef5491629d06eddb8e78ed01e50b175d2bd8 Mon Sep 17 00:00:00 2001 From: cloudwalk Date: Sat, 23 May 2020 22:22:30 +0000 Subject: [PATCH] Fix Windows-specific use-after-free causing crash after disconnecting This explicitly deregisters CSQC commands every time CSQC shuts down. This patch also avoids Z_Malloc'ing the cmd names for each CSQC command and avoids forcing a Cmd_AddCommand if the command exists. CSQC was force-adding a command even if it already existed, and when it did, it would pass a pointer from a mempool that would later get freed when CSQC shuts down, and Windows' strcasecmp doesn't like dangling pointers. The Z_Malloc wasn't much better because it could have caused a memory leak. So the best solution was to only pass the pointer but make sure the commands are freed when CSQC shuts down. git-svn-id: svn://svn.icculus.org/twilight/trunk/darkplaces@12558 d7cf8633-e32d-0410-b094-e92efae38249 --- clvm_cmds.c | 11 ----------- cmd.c | 13 ++++++++++--- cmd.h | 2 +- csprogs.c | 2 +- sbar.c | 7 +++++-- 5 files changed, 17 insertions(+), 18 deletions(-) diff --git a/clvm_cmds.c b/clvm_cmds.c index 25f3b195..eb297680 100644 --- a/clvm_cmds.c +++ b/clvm_cmds.c @@ -1623,20 +1623,9 @@ static void VM_CL_setlistener (prvm_prog_t *prog) //#352 void(string cmdname) registercommand (EXT_CSQC) static void VM_CL_registercmd (prvm_prog_t *prog) { - char *t; VM_SAFEPARMCOUNT(1, VM_CL_registercmd); if(!Cmd_Exists(&cmd_client, PRVM_G_STRING(OFS_PARM0))) - { - size_t alloclen; - - alloclen = strlen(PRVM_G_STRING(OFS_PARM0)) + 1; - t = (char *)Z_Malloc(alloclen); - memcpy(t, PRVM_G_STRING(OFS_PARM0), alloclen); - Cmd_AddCommand(&cmd_client, t, NULL, "console command created by QuakeC"); - } - else Cmd_AddCommand(&cmd_client, PRVM_G_STRING(OFS_PARM0), NULL, "console command created by QuakeC"); - } //#360 float() readbyte (EXT_CSQC) diff --git a/cmd.c b/cmd.c index 74f7eaff..fedfb13d 100644 --- a/cmd.c +++ b/cmd.c @@ -1982,11 +1982,18 @@ const char **Cmd_CompleteAliasBuildList (cmd_state_t *cmd, const char *partial) return buf; } -void Cmd_ClearCsqcFuncs (cmd_state_t *cmd) +// TODO: Make this more generic? +void Cmd_ClearCSQCCommands (cmd_state_t *cmd) { cmd_function_t *func; - for (func = cmd->userdefined->csqc_functions; func; func = func->next) - func->csqcfunc = false; + cmd_function_t **next = &cmd->userdefined->csqc_functions; + + while(*next) + { + func = *next; + *next = func->next; + Z_Free(func); + } } /* diff --git a/cmd.h b/cmd.h index 5a88ea91..98057fa8 100644 --- a/cmd.h +++ b/cmd.h @@ -248,7 +248,7 @@ void Cmd_ForwardToServer_f (cmd_state_t *cmd); /// enclosing quote marks are also put. qboolean Cmd_QuoteString(char *out, size_t outlen, const char *in, const char *quoteset, qboolean putquotes); -void Cmd_ClearCsqcFuncs (cmd_state_t *cmd); +void Cmd_ClearCSQCCommands (cmd_state_t *cmd); #endif diff --git a/csprogs.c b/csprogs.c index 3bc3495c..fb37cba0 100644 --- a/csprogs.c +++ b/csprogs.c @@ -1163,7 +1163,7 @@ void CL_VM_Init (void) void CL_VM_ShutDown (void) { prvm_prog_t *prog = CLVM_prog; - Cmd_ClearCsqcFuncs(&cmd_client); + Cmd_ClearCSQCCommands(&cmd_client); //Cvar_SetValueQuick(&csqc_progcrc, -1); //Cvar_SetValueQuick(&csqc_progsize, -1); if(!cl.csqc_loaded) diff --git a/sbar.c b/sbar.c index 40261c67..010925b7 100644 --- a/sbar.c +++ b/sbar.c @@ -357,8 +357,11 @@ static void sbar_newmap(void) void Sbar_Init (void) { - Cmd_AddCommand(&cmd_client, "+showscores", Sbar_ShowScores_f, "show scoreboard"); - Cmd_AddCommand(&cmd_client, "-showscores", Sbar_DontShowScores_f, "hide scoreboard"); + if(gamemode == GAME_NORMAL) // Workaround so Quake doesn't trample on Xonotic. + { + Cmd_AddCommand(&cmd_client, "+showscores", Sbar_ShowScores_f, "show scoreboard"); + Cmd_AddCommand(&cmd_client, "-showscores", Sbar_DontShowScores_f, "hide scoreboard"); + } Cvar_RegisterVariable(&showfps); Cvar_RegisterVariable(&showsound); Cvar_RegisterVariable(&showblur); -- 2.39.2