r1493 - in trunk/code: game qcommon server

[DONOTREPLY at icculus.org]

Author: ludwig
Date: 2009-01-17 18:09:58 -0500 (Sat, 17 Jan 2009)
New Revision: 1493

Modified:
   trunk/code/game/g_cmds.c
   trunk/code/qcommon/cmd.c
   trunk/code/qcommon/qcommon.h
   trunk/code/server/sv_client.c
Log:
security fix: prevent command injection via callvote

Modified: trunk/code/game/g_cmds.c
===================================================================
--- trunk/code/game/g_cmds.c	2009-01-13 07:57:03 UTC (rev 1492)
+++ trunk/code/game/g_cmds.c	2009-01-17 23:09:58 UTC (rev 1493)
@@ -1213,6 +1213,7 @@
 ==================
 */
 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 @@
 	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" ) ) {

Modified: trunk/code/qcommon/cmd.c
===================================================================
--- trunk/code/qcommon/cmd.c	2009-01-13 07:57:03 UTC (rev 1492)
+++ trunk/code/qcommon/cmd.c	2009-01-17 23:09:58 UTC (rev 1493)
@@ -434,6 +434,22 @@
 }
 
 /*
+   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
 

Modified: trunk/code/qcommon/qcommon.h
===================================================================
--- trunk/code/qcommon/qcommon.h	2009-01-13 07:57:03 UTC (rev 1492)
+++ trunk/code/qcommon/qcommon.h	2009-01-17 23:09:58 UTC (rev 1493)
@@ -434,6 +434,7 @@
 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.

Modified: trunk/code/server/sv_client.c
===================================================================
--- trunk/code/server/sv_client.c	2009-01-13 07:57:03 UTC (rev 1492)
+++ trunk/code/server/sv_client.c	2009-01-17 23:09:58 UTC (rev 1493)
@@ -1500,6 +1500,7 @@
 	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 );
 		}
 	}