From: havoc Date: Fri, 12 Jan 2007 03:40:35 +0000 (+0000) Subject: changed Cmd_AddCommand to only work for console commands, not client commands execute... X-Git-Tag: xonotic-v0.1.0preview~3719 X-Git-Url: https://git.rm.cloudns.org/?a=commitdiff_plain;h=3b27c0e3bc751cbf1f7c562c2bebfd3ec9b18f58;p=xonotic%2Fdarkplaces.git changed Cmd_AddCommand to only work for console commands, not client commands executed on the server, Cmd_AddCommand_WithClientCommand has been added to allow separate command functions for console commands and client commands, this got rid of a lot of cmd_source == src_command checks this refactoring fixes a security vulnerability in the clcommand builtin provided by KRIMZON_SV_PARSECLIENTCOMMAND, which was able to execute many commands on the server console, and that put the burden on the QC code to validate command safety, which was not intended in short: this fixes a remote console command execution vulnerability that affected a few games/mods git-svn-id: svn://svn.icculus.org/twilight/trunk/darkplaces@6685 d7cf8633-e32d-0410-b094-e92efae38249 --- diff --git a/cl_demo.c b/cl_demo.c index 627ccd12..3e7f1c50 100644 --- a/cl_demo.c +++ b/cl_demo.c @@ -216,9 +216,6 @@ stop recording a demo */ void CL_Stop_f (void) { - if (cmd_source != src_command) - return; - if (!cls.demorecording) { Con_Print("Not recording a demo.\n"); @@ -249,9 +246,6 @@ void CL_Record_f (void) int c, track; char name[MAX_OSPATH]; - if (cmd_source != src_command) - return; - c = Cmd_Argc(); if (c != 2 && c != 3 && c != 4) { @@ -317,9 +311,6 @@ void CL_PlayDemo_f (void) int c; qboolean neg = false; - if (cmd_source != src_command) - return; - if (Cmd_Argc() != 2) { Con_Print("play : plays a demo\n"); @@ -398,9 +389,6 @@ timedemo [demoname] */ void CL_TimeDemo_f (void) { - if (cmd_source != src_command) - return; - if (Cmd_Argc() != 2) { Con_Print("timedemo : gets demo speeds\n"); diff --git a/cmd.c b/cmd.c index 9797e01f..a59a46f6 100644 --- a/cmd.c +++ b/cmd.c @@ -482,7 +482,8 @@ typedef struct cmd_function_s struct cmd_function_s *next; const char *name; const char *description; - xcommand_t function; + xcommand_t consolefunction; + xcommand_t clientfunction; qboolean csqcfunc; } cmd_function_t; @@ -795,7 +796,7 @@ static void Cmd_TokenizeString (const char *text) Cmd_AddCommand ============ */ -void Cmd_AddCommand (const char *cmd_name, xcommand_t function, const char *description) +void Cmd_AddCommand_WithClientCommand (const char *cmd_name, xcommand_t consolefunction, xcommand_t clientfunction, const char *description) { cmd_function_t *cmd; cmd_function_t *prev, *current; @@ -812,7 +813,7 @@ void Cmd_AddCommand (const char *cmd_name, xcommand_t function, const char *desc { if (!strcmp (cmd_name, cmd->name)) { - if (function) + if (consolefunction || clientfunction) { Con_Printf("Cmd_AddCommand: %s already defined\n", cmd_name); return; @@ -827,9 +828,10 @@ void Cmd_AddCommand (const char *cmd_name, xcommand_t function, const char *desc cmd = (cmd_function_t *)Mem_Alloc(cmd_mempool, sizeof(cmd_function_t)); cmd->name = cmd_name; - cmd->function = function; + cmd->consolefunction = consolefunction; + cmd->clientfunction = clientfunction; cmd->description = description; - if(!function) //[515]: csqc + if(!consolefunction && !clientfunction) //[515]: csqc cmd->csqcfunc = true; cmd->next = cmd_functions; @@ -844,6 +846,11 @@ void Cmd_AddCommand (const char *cmd_name, xcommand_t function, const char *desc cmd->next = current; } +void Cmd_AddCommand (const char *cmd_name, xcommand_t function, const char *description) +{ + Cmd_AddCommand_WithClientCommand (cmd_name, function, NULL, description); +} + /* ============ Cmd_Exists @@ -1088,19 +1095,47 @@ void Cmd_ExecuteString (const char *text, cmd_source_t src) { if (!strcasecmp (cmd_argv[0],cmd->name)) { - if(cmd->function && !cmd->csqcfunc) - cmd->function (); - else - if(CL_VM_ConsoleCommand (text)) //[515]: csqc - return; + if (cmd->csqcfunc && CL_VM_ConsoleCommand (text)) //[515]: csqc + return; + switch (src) + { + case src_command: + if (cmd->consolefunction) + cmd->consolefunction (); + else if (cmd->clientfunction) + { + if (cls.state == ca_connected) + { + // forward remote commands to the server for execution + Cmd_ForwardToServer(); + } + else + Con_Printf("Can not send command \"%s\", not connected.\n", Cmd_Argv(0)); + } else - if(cmd->function) - cmd->function (); - cmd_tokenizebufferpos = oldpos; - return; + Con_Printf("Command \"%s\" can not be executed\n", Cmd_Argv(0)); + cmd_tokenizebufferpos = oldpos; + return; + case src_client: + if (cmd->clientfunction) + { + cmd->clientfunction (); + cmd_tokenizebufferpos = oldpos; + return; + } + break; + } + break; } } + // if it's a client command and no command was found, say so. + if (cmd_source == src_client) + { + Con_Printf("player \"%s\" tried to %s\n", host_client->name, text); + return; + } + // check alias for (a=cmd_alias ; a ; a=a->next) { diff --git a/cmd.h b/cmd.h index f7d9d17c..130e3cde 100644 --- a/cmd.h +++ b/cmd.h @@ -86,6 +86,7 @@ extern cmd_source_t cmd_source; void Cmd_Init (void); void Cmd_Shutdown (void); +void Cmd_AddCommand_WithClientCommand (const char *cmd_name, xcommand_t consolefunction, xcommand_t clientfunction, const char *description); void Cmd_AddCommand (const char *cmd_name, xcommand_t function, const char *description); // called by the init functions of other parts of the program to // register commands and functions to call for them. diff --git a/host_cmd.c b/host_cmd.c index d3665ed6..5f93996d 100644 --- a/host_cmd.c +++ b/host_cmd.c @@ -106,12 +106,6 @@ Sets client to godmode */ void Host_God_f (void) { - if (cmd_source == src_command) - { - Cmd_ForwardToServer (); - return; - } - if (!allowcheats) { SV_ClientPrint("No cheats allowed, use sv_cheats 1 and restart level to enable.\n"); @@ -127,12 +121,6 @@ void Host_God_f (void) void Host_Notarget_f (void) { - if (cmd_source == src_command) - { - Cmd_ForwardToServer (); - return; - } - if (!allowcheats) { SV_ClientPrint("No cheats allowed, use sv_cheats 1 and restart level to enable.\n"); @@ -150,12 +138,6 @@ qboolean noclip_anglehack; void Host_Noclip_f (void) { - if (cmd_source == src_command) - { - Cmd_ForwardToServer (); - return; - } - if (!allowcheats) { SV_ClientPrint("No cheats allowed, use sv_cheats 1 and restart level to enable.\n"); @@ -185,12 +167,6 @@ Sets client to flymode */ void Host_Fly_f (void) { - if (cmd_source == src_command) - { - Cmd_ForwardToServer (); - return; - } - if (!allowcheats) { SV_ClientPrint("No cheats allowed, use sv_cheats 1 and restart level to enable.\n"); @@ -278,9 +254,6 @@ void Host_Map_f (void) return; } - if (cmd_source != src_command) - return; - cls.demonum = -1; // stop demo loop in case this fails CL_Disconnect (); @@ -341,8 +314,6 @@ void Host_Changelevel_f (void) Host_Map_f(); return; } - if (cmd_source != src_command) - return; // remove menu key_dest = key_game; @@ -378,8 +349,6 @@ void Host_Restart_f (void) Con_Print("Only the server may restart\n"); return; } - if (cmd_source != src_command) - return; // remove menu key_dest = key_game; @@ -512,9 +481,6 @@ void Host_Savegame_f (void) int i; char comment[SAVEGAME_COMMENT_LENGTH+1]; - if (cmd_source != src_command) - return; - if (cls.state != ca_connected || !sv.active) { Con_Print("Not playing a local game.\n"); @@ -618,9 +584,6 @@ void Host_Loadgame_f (void) int version; float spawn_parms[NUM_SPAWN_PARMS]; - if (cmd_source != src_command) - return; - if (Cmd_Argc() != 2) { Con_Print("load : load a game\n"); @@ -1256,12 +1219,6 @@ Host_Kill_f */ void Host_Kill_f (void) { - if (cmd_source == src_command) - { - Cmd_ForwardToServer (); - return; - } - if (host_client->edict->fields.server->health <= 0) { SV_ClientPrint("Can't suicide -- already dead!\n"); @@ -1281,12 +1238,6 @@ Host_Pause_f */ void Host_Pause_f (void) { - - if (cmd_source == src_command) - { - Cmd_ForwardToServer (); - return; - } if (!pausable.integer) SV_ClientPrint("Pause not allowed.\n"); else @@ -1343,12 +1294,6 @@ Host_PreSpawn_f */ void Host_PreSpawn_f (void) { - if (cmd_source == src_command) - { - Con_Print("prespawn is not valid from the console\n"); - return; - } - if (host_client->spawned) { Con_Print("prespawn not valid -- already spawned\n"); @@ -1379,12 +1324,6 @@ void Host_Spawn_f (void) mfunction_t *f; int stats[MAX_CL_STATS]; - if (cmd_source == src_command) - { - Con_Print("spawn is not valid from the console\n"); - return; - } - if (host_client->spawned) { Con_Print("Spawn not valid -- already spawned\n"); @@ -1521,12 +1460,6 @@ Host_Begin_f */ void Host_Begin_f (void) { - if (cmd_source == src_command) - { - Con_Print("begin is not valid from the console\n"); - return; - } - host_client->spawned = true; } @@ -1548,7 +1481,7 @@ void Host_Kick_f (void) int i; qboolean byNumber = false; - if (cmd_source != src_command || !sv.active) + if (!sv.active) return; SV_VM_Begin(); @@ -1632,12 +1565,6 @@ void Host_Give_f (void) int v; prvm_eval_t *val; - if (cmd_source == src_command) - { - Cmd_ForwardToServer (); - return; - } - if (!allowcheats) { SV_ClientPrint("No cheats allowed, use sv_cheats 1 and restart level to enable.\n"); @@ -2345,11 +2272,6 @@ void Host_Pings_f (void) int i, j, ping, packetloss; char temp[128]; - if (cmd_source == src_command) - { - Cmd_ForwardToServer (); - return; - } if (!host_client->netconnection) return; @@ -2409,23 +2331,23 @@ void Host_InitCommands (void) { dpsnprintf(cls.userinfo, sizeof(cls.userinfo), "\\name\\player\\team\\none\\topcolor\\0\\bottomcolor\\0\\rate\\10000\\msg\\1\\noaim\\1\\*ver\\%s", engineversion); - Cmd_AddCommand ("status", Host_Status_f, "print server status information"); + Cmd_AddCommand_WithClientCommand ("status", Host_Status_f, Host_Status_f, "print server status information"); Cmd_AddCommand ("quit", Host_Quit_f, "quit the game"); if (gamemode == GAME_NEHAHRA) { - Cmd_AddCommand ("max", Host_God_f, "god mode (invulnerability)"); - Cmd_AddCommand ("monster", Host_Notarget_f, "notarget mode (monsters do not see you)"); - Cmd_AddCommand ("scrag", Host_Fly_f, "fly mode (flight)"); - Cmd_AddCommand ("wraith", Host_Noclip_f, "noclip mode (flight without collisions, move through walls)"); - Cmd_AddCommand ("gimme", Host_Give_f, "alter inventory"); + Cmd_AddCommand_WithClientCommand ("max", NULL, Host_God_f, "god mode (invulnerability)"); + Cmd_AddCommand_WithClientCommand ("monster", NULL, Host_Notarget_f, "notarget mode (monsters do not see you)"); + Cmd_AddCommand_WithClientCommand ("scrag", NULL, Host_Fly_f, "fly mode (flight)"); + Cmd_AddCommand_WithClientCommand ("wraith", NULL, Host_Noclip_f, "noclip mode (flight without collisions, move through walls)"); + Cmd_AddCommand_WithClientCommand ("gimme", NULL, Host_Give_f, "alter inventory"); } else { - Cmd_AddCommand ("god", Host_God_f, "god mode (invulnerability)"); - Cmd_AddCommand ("notarget", Host_Notarget_f, "notarget mode (monsters do not see you)"); - Cmd_AddCommand ("fly", Host_Fly_f, "fly mode (flight)"); - Cmd_AddCommand ("noclip", Host_Noclip_f, "noclip mode (flight without collisions, move through walls)"); - Cmd_AddCommand ("give", Host_Give_f, "alter inventory"); + Cmd_AddCommand_WithClientCommand ("god", NULL, Host_God_f, "god mode (invulnerability)"); + Cmd_AddCommand_WithClientCommand ("notarget", NULL, Host_Notarget_f, "notarget mode (monsters do not see you)"); + Cmd_AddCommand_WithClientCommand ("fly", NULL, Host_Fly_f, "fly mode (flight)"); + Cmd_AddCommand_WithClientCommand ("noclip", NULL, Host_Noclip_f, "noclip mode (flight without collisions, move through walls)"); + Cmd_AddCommand_WithClientCommand ("give", NULL, Host_Give_f, "alter inventory"); } Cmd_AddCommand ("map", Host_Map_f, "kick everyone off the server and start a new level"); Cmd_AddCommand ("restart", Host_Restart_f, "restart current level"); @@ -2433,13 +2355,13 @@ void Host_InitCommands (void) Cmd_AddCommand ("connect", Host_Connect_f, "connect to a server by IP address or hostname"); Cmd_AddCommand ("reconnect", Host_Reconnect_f, "reset signon level in preparation for a new level (do not use)"); Cmd_AddCommand ("version", Host_Version_f, "print engine version"); - Cmd_AddCommand ("say", Host_Say_f, "send a chat message to everyone on the server"); - Cmd_AddCommand ("say_team", Host_Say_Team_f, "send a chat message to your team on the server"); - Cmd_AddCommand ("tell", Host_Tell_f, "send a chat message to only one person on the server"); - Cmd_AddCommand ("kill", Host_Kill_f, "die instantly"); - Cmd_AddCommand ("pause", Host_Pause_f, "pause the game (if the server allows pausing)"); + Cmd_AddCommand_WithClientCommand ("say", Host_Say_f, Host_Say_f, "send a chat message to everyone on the server"); + Cmd_AddCommand_WithClientCommand ("say_team", Host_Say_Team_f, Host_Say_Team_f, "send a chat message to your team on the server"); + Cmd_AddCommand_WithClientCommand ("tell", Host_Tell_f, Host_Tell_f, "send a chat message to only one person on the server"); + Cmd_AddCommand_WithClientCommand ("kill", NULL, Host_Kill_f, "die instantly"); + Cmd_AddCommand_WithClientCommand ("pause", NULL, Host_Pause_f, "pause the game (if the server allows pausing)"); Cmd_AddCommand ("kick", Host_Kick_f, "kick a player off the server by number or name"); - Cmd_AddCommand ("ping", Host_Ping_f, "print ping times of all players on the server"); + Cmd_AddCommand_WithClientCommand ("ping", Host_Ping_f, Host_Ping_f, "print ping times of all players on the server"); Cmd_AddCommand ("load", Host_Loadgame_f, "load a saved game file"); Cmd_AddCommand ("save", Host_Savegame_f, "save the game to a file"); @@ -2453,26 +2375,26 @@ void Host_InitCommands (void) Cmd_AddCommand ("viewprev", Host_Viewprev_f, "change to previous animation frame of viewthing entity in current level"); Cvar_RegisterVariable (&cl_name); - Cmd_AddCommand ("name", Host_Name_f, "change your player name"); + Cmd_AddCommand_WithClientCommand ("name", Host_Name_f, Host_Name_f, "change your player name"); Cvar_RegisterVariable (&cl_color); - Cmd_AddCommand ("color", Host_Color_f, "change your player shirt and pants colors"); + Cmd_AddCommand_WithClientCommand ("color", Host_Color_f, Host_Color_f, "change your player shirt and pants colors"); Cvar_RegisterVariable (&cl_rate); - Cmd_AddCommand ("rate", Host_Rate_f, "change your network connection speed"); + Cmd_AddCommand_WithClientCommand ("rate", Host_Rate_f, Host_Rate_f, "change your network connection speed"); if (gamemode == GAME_NEHAHRA) { Cvar_RegisterVariable (&cl_pmodel); - Cmd_AddCommand ("pmodel", Host_PModel_f, "change your player model choice (Nehahra specific)"); + Cmd_AddCommand_WithClientCommand ("pmodel", Host_PModel_f, Host_PModel_f, "change your player model choice (Nehahra specific)"); } // BLACK: This isnt game specific anymore (it was GAME_NEXUIZ at first) Cvar_RegisterVariable (&cl_playermodel); - Cmd_AddCommand ("playermodel", Host_Playermodel_f, "change your player model"); + Cmd_AddCommand_WithClientCommand ("playermodel", Host_Playermodel_f, Host_Playermodel_f, "change your player model"); Cvar_RegisterVariable (&cl_playerskin); - Cmd_AddCommand ("playerskin", Host_Playerskin_f, "change your player skin number"); + Cmd_AddCommand_WithClientCommand ("playerskin", Host_Playerskin_f, Host_Playerskin_f, "change your player skin number"); - Cmd_AddCommand ("prespawn", Host_PreSpawn_f, "signon 1 (client acknowledges that server information has been received)"); - Cmd_AddCommand ("spawn", Host_Spawn_f, "signon 2 (client has sent player information, and is asking server to send scoreboard rankings)"); - Cmd_AddCommand ("begin", Host_Begin_f, "signon 3 (client asks server to start sending entities, and will go to signon 4 (playing) when the first entity update is received)"); + Cmd_AddCommand_WithClientCommand ("prespawn", NULL, Host_PreSpawn_f, "signon 1 (client acknowledges that server information has been received)"); + Cmd_AddCommand_WithClientCommand ("spawn", NULL, Host_Spawn_f, "signon 2 (client has sent player information, and is asking server to send scoreboard rankings)"); + Cmd_AddCommand_WithClientCommand ("begin", NULL, Host_Begin_f, "signon 3 (client asks server to start sending entities, and will go to signon 4 (playing) when the first entity update is received)"); Cmd_AddCommand ("maxplayers", MaxPlayers_f, "sets limit on how many players (or bots) may be connected to the server at once"); Cmd_AddCommand ("sendcvar", Host_SendCvar_f, "sends the value of a cvar to the server as a sentcvar command, for use by QuakeC"); // By [515] @@ -2489,7 +2411,7 @@ void Host_InitCommands (void) Cmd_AddCommand ("topcolor", Host_TopColor_f, "QW command to set top color without changing bottom color"); Cmd_AddCommand ("bottomcolor", Host_BottomColor_f, "QW command to set bottom color without changing top color"); - Cmd_AddCommand ("pings", Host_Pings_f, "command sent by clients to request updated ping and packetloss of players on scoreboard (originally from QW, but also used on NQ servers)"); + Cmd_AddCommand_WithClientCommand ("pings", NULL, Host_Pings_f, "command sent by clients to request updated ping and packetloss of players on scoreboard (originally from QW, but also used on NQ servers)"); Cmd_AddCommand ("pingplreport", Host_PingPLReport_f, "command sent by server containing client ping and packet loss values for scoreboard, triggered by pings command from client (not used by QW servers)"); Cvar_RegisterVariable (&team); diff --git a/sv_user.c b/sv_user.c index e8998013..3d2e78c5 100644 --- a/sv_user.c +++ b/sv_user.c @@ -696,27 +696,8 @@ void SV_ReadClientMessage(void) prog->globals.server->self = PRVM_EDICT_TO_PROG(host_client->edict); PRVM_ExecuteProgram ((func_t)(SV_ParseClientCommandQC - prog->functions), "QC function SV_ParseClientCommand is missing"); } - else if (strncasecmp(s, "status", 6) == 0 - || strncasecmp(s, "name", 4) == 0 - || strncasecmp(s, "say", 3) == 0 - || strncasecmp(s, "say_team", 8) == 0 - || strncasecmp(s, "tell", 4) == 0 - || strncasecmp(s, "color", 5) == 0 - || strncasecmp(s, "kill", 4) == 0 - || strncasecmp(s, "pause", 5) == 0 - || strncasecmp(s, "kick", 4) == 0 - || strncasecmp(s, "ping", 4) == 0 - || strncasecmp(s, "pings", 5) == 0 - || strncasecmp(s, "ban", 3) == 0 - || strncasecmp(s, "pmodel", 6) == 0 - || strncasecmp(s, "rate", 4) == 0 - || strncasecmp(s, "playermodel", 11) == 0 - || strncasecmp(s, "playerskin", 10) == 0 - || (gamemode == GAME_NEHAHRA && (strncasecmp(s, "max", 3) == 0 || strncasecmp(s, "monster", 7) == 0 || strncasecmp(s, "scrag", 5) == 0 || strncasecmp(s, "gimme", 5) == 0 || strncasecmp(s, "wraith", 6) == 0)) - || (gamemode != GAME_NEHAHRA && (strncasecmp(s, "god", 3) == 0 || strncasecmp(s, "notarget", 8) == 0 || strncasecmp(s, "fly", 3) == 0 || strncasecmp(s, "give", 4) == 0 || strncasecmp(s, "noclip", 6) == 0))) - Cmd_ExecuteString (s, src_client); else - Con_Printf("%s tried to %s\n", host_client->name, s); + Cmd_ExecuteString (s, src_client); break; case clc_disconnect: