From b900e8e57ce8be0dfef6c4e79601a071b0932a46 Mon Sep 17 00:00:00 2001 From: ludwig Date: Sat, 17 Jan 2009 23:09:58 +0000 Subject: security fix: prevent command injection via callvote git-svn-id: svn://svn.icculus.org/quake3/trunk@1493 edf5b092-35ff-0310-97b2-ce42778d08ea --- code/game/g_cmds.c | 14 +++++++++++--- code/qcommon/cmd.c | 16 ++++++++++++++++ code/qcommon/qcommon.h | 1 + code/server/sv_client.c | 1 + 4 files changed, 29 insertions(+), 3 deletions(-) (limited to 'code') diff --git a/code/game/g_cmds.c b/code/game/g_cmds.c index b8fa2b3..1ecb0cb 100644 --- a/code/game/g_cmds.c +++ b/code/game/g_cmds.c @@ -1213,6 +1213,7 @@ Cmd_CallVote_f ================== */ void Cmd_CallVote_f( gentity_t *ent ) { + char* c; int i; char arg1[MAX_STRING_TOKENS]; char arg2[MAX_STRING_TOKENS]; @@ -1239,9 +1240,16 @@ void Cmd_CallVote_f( gentity_t *ent ) { trap_Argv( 1, arg1, sizeof( arg1 ) ); trap_Argv( 2, arg2, sizeof( arg2 ) ); - if( strchr( arg1, ';' ) || strchr( arg2, ';' ) ) { - trap_SendServerCommand( ent-g_entities, "print \"Invalid vote string.\n\"" ); - return; + // check for command separators in arg2 + for( c = arg2; *c; ++c) { + switch(*c) { + case '\n': + case '\r': + case ';': + trap_SendServerCommand( ent-g_entities, "print \"Invalid vote string.\n\"" ); + return; + break; + } } if ( !Q_stricmp( arg1, "map_restart" ) ) { diff --git a/code/qcommon/cmd.c b/code/qcommon/cmd.c index 08af301..8b14c2a 100644 --- a/code/qcommon/cmd.c +++ b/code/qcommon/cmd.c @@ -433,6 +433,22 @@ char *Cmd_Cmd(void) return cmd_cmd; } +/* + Replace command separators with space to prevent interpretation + This is a hack to protect buggy qvms + https://bugzilla.icculus.org/show_bug.cgi?id=3593 +*/ +void Cmd_Args_Sanitize( void ) { + int i; + for ( i = 1 ; i < cmd_argc ; i++ ) { + char* c = cmd_argv[i]; + while ((c = strpbrk(c, "\n\r;"))) { + *c = ' '; + ++c; + } + } +} + /* ============ Cmd_TokenizeString diff --git a/code/qcommon/qcommon.h b/code/qcommon/qcommon.h index 6a264d3..34cb2e9 100644 --- a/code/qcommon/qcommon.h +++ b/code/qcommon/qcommon.h @@ -434,6 +434,7 @@ char *Cmd_Args (void); char *Cmd_ArgsFrom( int arg ); void Cmd_ArgsBuffer( char *buffer, int bufferLength ); char *Cmd_Cmd (void); +void Cmd_Args_Sanitize( void ); // The functions that execute commands get their parameters with these // functions. Cmd_Argv () will return an empty string, not a NULL // if arg > argc, so string operations are allways safe. diff --git a/code/server/sv_client.c b/code/server/sv_client.c index 5554ebf..01f4d8b 100644 --- a/code/server/sv_client.c +++ b/code/server/sv_client.c @@ -1500,6 +1500,7 @@ void SV_ExecuteClientCommand( client_t *cl, const char *s, qboolean clientOK ) { if (clientOK) { // pass unknown strings to the game if (!u->name && sv.state == SS_GAME) { + Cmd_Args_Sanitize(); VM_Call( gvm, GAME_CLIENT_COMMAND, cl - svs.clients ); } } -- cgit v1.2.3