security fix: prevent command injection via callvote

Luigi Auriemma q3cbufexec

from Ludwig Nussel in ioquake3
svn 1493 git f5aae78481d71307a0b874b1f17ecdead1469392

security fix: prevent command injection via callvote

from Thilo Schulz in ioquake3
svn 1838 git cf791d14c58f536eec8220d93fb9af443f8837e9

- Fix bug #4769 remote server crash

codemp/game/g_cmds.c

@@ -1972,6 +1972,7 @@ Cmd_CallVote_f
 extern void SiegeClearSwitchData(void); //g_saga.c
 const char *G_GetArenaInfoByMap( const char *map );
 void Cmd_CallVote_f( gentity_t *ent ) {
+	char*	c;
 	int		i;
 	char	arg1[MAX_STRING_TOKENS];
 	char	arg2[MAX_STRING_TOKENS];
@@ -2012,6 +2013,18 @@ void Cmd_CallVote_f( gentity_t *ent ) {
 		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" ) ) {
 	} else if ( !Q_stricmp( arg1, "nextmap" ) ) {
 	} else if ( !Q_stricmp( arg1, "map" ) ) {

codemp/qcommon/cmd_common.cpp

@@ -384,6 +384,30 @@ void	Cmd_ArgsBuffer( char *buffer, int bufferLength ) {
 	Q_strncpyz( buffer, Cmd_Args(), bufferLength );
 }
 
+/*
+   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
+   https://bugzilla.icculus.org/show_bug.cgi?id=4769
+*/
+
+void Cmd_Args_Sanitize(void)
+{
+	int i;
+
+	for(i = 1; i < cmd_argc; i++)
+	{
+		char *c = cmd_argv[i];
+		
+		if(strlen(c) > MAX_CVAR_VALUE_STRING - 1)
+			c[MAX_CVAR_VALUE_STRING - 1] = '\0';
+		
+		while ((c = strpbrk(c, "\n\r;"))) {
+			*c = ' ';
+			++c;
+		}
+	}
+}
 
 /*
 ============

codemp/qcommon/qcommon.h

@@ -382,6 +382,7 @@ void	Cmd_ArgvBuffer( int arg, char *buffer, int bufferLength );
 char	*Cmd_Args (void);
 char	*Cmd_ArgsFrom( int arg );
 void	Cmd_ArgsBuffer( char *buffer, int bufferLength );
+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.

codemp/server/sv_client.cpp

@@ -1611,6 +1611,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 );
 		}
 	}