[quake3-commits] r1916 - in trunk/code: AL client qcommon server

DONOTREPLY at icculus.org DONOTREPLY at icculus.org
Mon Mar 7 20:39:34 EST 2011


Author: thilo
Date: 2011-03-07 20:39:34 -0500 (Mon, 07 Mar 2011)
New Revision: 1916

Modified:
   trunk/code/AL/alctypes.h
   trunk/code/client/cl_cgame.c
   trunk/code/client/cl_parse.c
   trunk/code/client/cl_ui.c
   trunk/code/client/snd_openal.c
   trunk/code/qcommon/cmd.c
   trunk/code/qcommon/cvar.c
   trunk/code/qcommon/files.c
   trunk/code/qcommon/q_shared.h
   trunk/code/qcommon/qcommon.h
   trunk/code/server/sv_game.c
   trunk/code/server/sv_main.c
Log:
(#3767) Some protection from malicious qvms - patches and ideas by Amanieu d'Antras and Ben Millwood


Modified: trunk/code/AL/alctypes.h
===================================================================
--- trunk/code/AL/alctypes.h	2011-03-08 01:37:28 UTC (rev 1915)
+++ trunk/code/AL/alctypes.h	2011-03-08 01:39:34 UTC (rev 1916)
@@ -123,6 +123,7 @@
  */
 #define ALC_DEFAULT_DEVICE_SPECIFIER             0x1004
 #define ALC_DEVICE_SPECIFIER                     0x1005
+#define ALC_ALL_DEVICES_SPECIFIER                0x1013
 #define ALC_EXTENSIONS                           0x1006
 
 #define ALC_MAJOR_VERSION                        0x1000

Modified: trunk/code/client/cl_cgame.c
===================================================================
--- trunk/code/client/cl_cgame.c	2011-03-08 01:37:28 UTC (rev 1915)
+++ trunk/code/client/cl_cgame.c	2011-03-08 01:39:34 UTC (rev 1916)
@@ -431,7 +431,7 @@
 		Cvar_Update( VMA(1) );
 		return 0;
 	case CG_CVAR_SET:
-		Cvar_Set( VMA(1), VMA(2) );
+		Cvar_SetSafe( VMA(1), VMA(2) );
 		return 0;
 	case CG_CVAR_VARIABLESTRINGBUFFER:
 		Cvar_VariableStringBuffer( VMA(1), VMA(2), args[3] );
@@ -464,7 +464,7 @@
 		CL_AddCgameCommand( VMA(1) );
 		return 0;
 	case CG_REMOVECOMMAND:
-		Cmd_RemoveCommand( VMA(1) );
+		Cmd_RemoveCommandSafe( VMA(1) );
 		return 0;
 	case CG_SENDCLIENTCOMMAND:
 		CL_AddReliableCommand(VMA(1), qfalse);

Modified: trunk/code/client/cl_parse.c
===================================================================
--- trunk/code/client/cl_parse.c	2011-03-08 01:37:28 UTC (rev 1915)
+++ trunk/code/client/cl_parse.c	2011-03-08 01:39:34 UTC (rev 1916)
@@ -416,13 +416,13 @@
 		else
 		{
 			// If this cvar may not be modified by a server discard the value.
-			if(!(cvar_flags & (CVAR_SYSTEMINFO | CVAR_SERVER_CREATED)))
+			if(!(cvar_flags & (CVAR_SYSTEMINFO | CVAR_SERVER_CREATED | CVAR_USER_CREATED)))
 			{
 				Com_Printf(S_COLOR_YELLOW "WARNING: server is not allowed to set %s=%s\n", key, value);
 				continue;
 			}
 
-			Cvar_Set(key, value);
+			Cvar_SetSafe(key, value);
 		}
 	}
 	// if game folder should not be set and it is set at the client side

Modified: trunk/code/client/cl_ui.c
===================================================================
--- trunk/code/client/cl_ui.c	2011-03-08 01:37:28 UTC (rev 1915)
+++ trunk/code/client/cl_ui.c	2011-03-08 01:39:34 UTC (rev 1916)
@@ -731,7 +731,7 @@
 		return 0;
 
 	case UI_CVAR_SET:
-		Cvar_Set( VMA(1), VMA(2) );
+		Cvar_SetSafe( VMA(1), VMA(2) );
 		return 0;
 
 	case UI_CVAR_VARIABLEVALUE:
@@ -742,7 +742,7 @@
 		return 0;
 
 	case UI_CVAR_SETVALUE:
-		Cvar_SetValue( VMA(1), VMF(2) );
+		Cvar_SetValueSafe( VMA(1), VMF(2) );
 		return 0;
 
 	case UI_CVAR_RESET:

Modified: trunk/code/client/snd_openal.c
===================================================================
--- trunk/code/client/snd_openal.c	2011-03-08 01:37:28 UTC (rev 1915)
+++ trunk/code/client/snd_openal.c	2011-03-08 01:39:34 UTC (rev 1916)
@@ -41,7 +41,9 @@
 cvar_t *s_alGraceDistance;
 cvar_t *s_alDriver;
 cvar_t *s_alDevice;
+cvar_t *s_alInputDevice;
 cvar_t *s_alAvailableDevices;
+cvar_t *s_alAvailableInputDevices;
 
 /*
 =================
@@ -2278,11 +2280,17 @@
 	Com_Printf( "  ALC Extensions: %s\n", qalcGetString( alDevice, ALC_EXTENSIONS ) );
 	if(qalcIsExtensionPresent(NULL, "ALC_ENUMERATION_EXT"))
 	{
-		Com_Printf("  Device:     %s\n", qalcGetString(alDevice, ALC_DEVICE_SPECIFIER));
+		Com_Printf("  Device:      %s\n", qalcGetString(alDevice, ALC_DEVICE_SPECIFIER));
 		Com_Printf("Available Devices:\n%s", s_alAvailableDevices->string);
+#ifdef USE_VOIP
+		Com_Printf("Input Device:  %s\n", qalcGetString(alCaptureDevice, ALC_DEVICE_SPECIFIER));
+		Com_Printf("Available Input Devices:\n%s", s_alAvailableInputDevices->string);
+#endif
 	}
 }
 
+
+
 /*
 =================
 S_AL_Shutdown
@@ -2331,6 +2339,7 @@
 {
 #ifdef USE_OPENAL
 	const char* device = NULL;
+	const char* inputdevice = NULL;
 	int i;
 
 	if( !si ) {
@@ -2355,6 +2364,7 @@
 	s_alGraceDistance = Cvar_Get("s_alGraceDistance", "512", CVAR_CHEAT);
 
 	s_alDriver = Cvar_Get( "s_alDriver", ALDRIVER_DEFAULT, CVAR_ARCHIVE | CVAR_LATCH );
+	s_alInputDevice = Cvar_Get( "s_alInputDevice", ALDRIVER_DEFAULT, CVAR_ARCHIVE | CVAR_LATCH );
 
 	s_alDevice = Cvar_Get("s_alDevice", "", CVAR_ARCHIVE | CVAR_LATCH);
 
@@ -2369,6 +2379,10 @@
 	if(device && !*device)
 		device = NULL;
 
+	inputdevice = s_alInputDevice->string;
+	if(inputdevice && !*inputdevice)
+		inputdevice = NULL;
+
 	// Device enumeration support (extension is implemented reasonably only on Windows right now).
 	if(qalcIsExtensionPresent(NULL, "ALC_ENUMERATION_EXT"))
 	{
@@ -2378,7 +2392,7 @@
 		int curlen;
 		
 		// get all available devices + the default device name.
-		devicelist = qalcGetString(NULL, ALC_DEVICE_SPECIFIER);
+		devicelist = qalcGetString(NULL, ALC_ALL_DEVICES_SPECIFIER);
 		defaultdevice = qalcGetString(NULL, ALC_DEFAULT_DEVICE_SPECIFIER);
 
 #ifdef _WIN32
@@ -2468,13 +2482,36 @@
 		}
 		else
 		{
+			char inputdevicenames[1024] = "";
+			const char *inputdevicelist;
+			const char *defaultinputdevice;
+			int curlen;
+
+			// get all available input devices + the default input device name.
+			inputdevicelist = qalcGetString(NULL, ALC_CAPTURE_DEVICE_SPECIFIER);
+			defaultinputdevice = qalcGetString(NULL, ALC_CAPTURE_DEFAULT_DEVICE_SPECIFIER);
+
+			// dump a list of available devices to a cvar for the user to see.
+			while((curlen = strlen(inputdevicelist)))
+			{
+				Q_strcat(inputdevicenames, sizeof(inputdevicenames), inputdevicelist);
+				Q_strcat(inputdevicenames, sizeof(inputdevicenames), "\n");
+				inputdevicelist += curlen + 1;
+			}
+
+			s_alAvailableInputDevices = Cvar_Get("s_alAvailableInputDevices", inputdevicenames, CVAR_ROM | CVAR_NORESTART);
+
 			// !!! FIXME: 8000Hz is what Speex narrowband mode needs, but we
 			// !!! FIXME:  should probably open the capture device after
 			// !!! FIXME:  initializing Speex so we can change to wideband
 			// !!! FIXME:  if we like.
-			Com_Printf("OpenAL default capture device is '%s'\n",
-			           qalcGetString(NULL, ALC_CAPTURE_DEFAULT_DEVICE_SPECIFIER));
-			alCaptureDevice = qalcCaptureOpenDevice(NULL, 8000, AL_FORMAT_MONO16, 4096);
+			Com_Printf("OpenAL default capture device is '%s'\n", defaultinputdevice);
+			alCaptureDevice = qalcCaptureOpenDevice(inputdevice, 8000, AL_FORMAT_MONO16, 4096);
+			if( !alCaptureDevice && inputdevice )
+			{
+				Com_Printf( "Failed to open OpenAL Input device '%s', trying default.\n", inputdevice );
+				alCaptureDevice = qalcCaptureOpenDevice(NULL, 8000, AL_FORMAT_MONO16, 4096);
+			}
 			Com_Printf( "OpenAL capture device %s.\n",
 			            (alCaptureDevice == NULL) ? "failed to open" : "opened");
 		}

Modified: trunk/code/qcommon/cmd.c
===================================================================
--- trunk/code/qcommon/cmd.c	2011-03-08 01:37:28 UTC (rev 1915)
+++ trunk/code/qcommon/cmd.c	2011-03-08 01:39:34 UTC (rev 1916)
@@ -590,6 +590,20 @@
 
 /*
 ============
+Cmd_FindCommand
+============
+*/
+cmd_function_t *Cmd_FindCommand( const char *cmd_name )
+{
+	cmd_function_t *cmd;
+	for( cmd = cmd_functions; cmd; cmd = cmd->next )
+		if( !Q_stricmp( cmd_name, cmd->name ) )
+			return cmd;
+	return NULL;
+}
+
+/*
+============
 Cmd_AddCommand
 ============
 */
@@ -597,14 +611,12 @@
 	cmd_function_t	*cmd;
 	
 	// fail if the command already exists
-	for ( cmd = cmd_functions ; cmd ; cmd=cmd->next ) {
-		if ( !strcmp( cmd_name, cmd->name ) ) {
-			// allow completion-only commands to be silently doubled
-			if ( function != NULL ) {
-				Com_Printf ("Cmd_AddCommand: %s already defined\n", cmd_name);
-			}
-			return;
-		}
+	if( Cmd_FindCommand( cmd_name ) )
+	{
+		// allow completion-only commands to be silently doubled
+		if( function != NULL )
+			Com_Printf( "Cmd_AddCommand: %s already defined\n", cmd_name );
+		return;
 	}
 
 	// use a small malloc to avoid zone fragmentation
@@ -658,7 +670,29 @@
 	}
 }
 
+/*
+============
+Cmd_RemoveCommandSafe
 
+Only remove commands with no associated function
+============
+*/
+void Cmd_RemoveCommandSafe( const char *cmd_name )
+{
+	cmd_function_t *cmd = Cmd_FindCommand( cmd_name );
+
+	if( !cmd )
+		return;
+	if( cmd->function )
+	{
+		Com_Error( ERR_DROP, "Restricted source tried to remove "
+			"system command \"%s\"\n", cmd_name );
+		return;
+	}
+
+	Cmd_RemoveCommand( cmd_name );
+}
+
 /*
 ============
 Cmd_CommandCompletion

Modified: trunk/code/qcommon/cvar.c
===================================================================
--- trunk/code/qcommon/cvar.c	2011-03-08 01:37:28 UTC (rev 1915)
+++ trunk/code/qcommon/cvar.c	2011-03-08 01:39:34 UTC (rev 1916)
@@ -363,6 +363,18 @@
 				flags &= ~CVAR_VM_CREATED;
 		}
 		
+		// Make sure servers cannot mark engine-added variables as SERVER_CREATED
+		if(var->flags & CVAR_SERVER_CREATED)
+		{
+			if(!(flags & CVAR_SERVER_CREATED))
+				var->flags &= ~CVAR_SERVER_CREATED;
+		}
+		else
+		{
+			if(flags & CVAR_SERVER_CREATED)
+				flags &= ~CVAR_SERVER_CREATED;
+		}
+		
 		var->flags |= flags;
 
 		// only allow one non-empty reset string without a warning
@@ -612,6 +624,28 @@
 
 /*
 ============
+Cvar_SetSafe
+============
+*/
+void Cvar_SetSafe( const char *var_name, const char *value )
+{
+	int flags = Cvar_Flags( var_name );
+
+	if( flags != CVAR_NONEXISTENT && flags & CVAR_PROTECTED )
+	{
+		if( value )
+			Com_Error( ERR_DROP, "Restricted source tried to set "
+				"\"%s\" to \"%s\"\n", var_name, value );
+		else
+			Com_Error( ERR_DROP, "Restricted source tried to "
+				"modify \"%s\"\n", var_name );
+		return;
+	}
+	Cvar_Set( var_name, value );
+}
+
+/*
+============
 Cvar_SetLatched
 ============
 */
@@ -635,7 +669,22 @@
 	Cvar_Set (var_name, val);
 }
 
+/*
+============
+Cvar_SetValueSafe
+============
+*/
+void Cvar_SetValueSafe( const char *var_name, float value )
+{
+	char val[32];
 
+	if( Q_isintegral( value ) )
+		Com_sprintf( val, sizeof(val), "%i", (int)value );
+	else
+		Com_sprintf( val, sizeof(val), "%f", value );
+	Cvar_SetSafe( var_name, val );
+}
+
 /*
 ============
 Cvar_Reset

Modified: trunk/code/qcommon/files.c
===================================================================
--- trunk/code/qcommon/files.c	2011-03-08 01:37:28 UTC (rev 1915)
+++ trunk/code/qcommon/files.c	2011-03-08 01:39:34 UTC (rev 1916)
@@ -2910,13 +2910,13 @@
 	fs_packFiles = 0;
 
 	fs_debug = Cvar_Get( "fs_debug", "0", 0 );
-	fs_basepath = Cvar_Get ("fs_basepath", Sys_DefaultInstallPath(), CVAR_INIT );
+	fs_basepath = Cvar_Get ("fs_basepath", Sys_DefaultInstallPath(), CVAR_INIT|CVAR_PROTECTED );
 	fs_basegame = Cvar_Get ("fs_basegame", "", CVAR_INIT );
 	homePath = Sys_DefaultHomePath();
 	if (!homePath || !homePath[0]) {
 		homePath = fs_basepath->string;
 	}
-	fs_homepath = Cvar_Get ("fs_homepath", homePath, CVAR_INIT );
+	fs_homepath = Cvar_Get ("fs_homepath", homePath, CVAR_INIT|CVAR_PROTECTED );
 	fs_gamedirvar = Cvar_Get ("fs_game", "", CVAR_INIT|CVAR_SYSTEMINFO );
 
 	// add search path elements in reverse priority order
@@ -2926,7 +2926,7 @@
 	// fs_homepath is somewhat particular to *nix systems, only add if relevant
 	
 	#ifdef MACOS_X
-	fs_apppath = Cvar_Get ("fs_apppath", Sys_DefaultAppPath(), CVAR_INIT );
+	fs_apppath = Cvar_Get ("fs_apppath", Sys_DefaultAppPath(), CVAR_INIT|CVAR_PROTECTED );
 	// Make MacOSX also include the base path included with the .app bundle
 	if (fs_apppath->string[0])
 		FS_AddGameDirectory(fs_apppath->string, gameName);

Modified: trunk/code/qcommon/q_shared.h
===================================================================
--- trunk/code/qcommon/q_shared.h	2011-03-08 01:37:28 UTC (rev 1915)
+++ trunk/code/qcommon/q_shared.h	2011-03-08 01:39:34 UTC (rev 1916)
@@ -815,6 +815,7 @@
 
 #define CVAR_SERVER_CREATED	0x0800	// cvar was created by a server the client connected to.
 #define CVAR_VM_CREATED		0x1000	// cvar was created exclusively in one of the VMs.
+#define CVAR_PROTECTED		0x2000	// prevent modifying this var from VMs or the server
 #define CVAR_NONEXISTENT	0xFFFFFFFF	// Cvar doesn't exist.
 
 // nothing outside the Cvar_*() functions should modify these fields!

Modified: trunk/code/qcommon/qcommon.h
===================================================================
--- trunk/code/qcommon/qcommon.h	2011-03-08 01:37:28 UTC (rev 1915)
+++ trunk/code/qcommon/qcommon.h	2011-03-08 01:39:34 UTC (rev 1916)
@@ -430,6 +430,9 @@
 
 typedef void (*completionFunc_t)( char *args, int argNum );
 
+// don't allow VMs to remove system commands
+void	Cmd_RemoveCommandSafe( const char *cmd_name );
+
 void	Cmd_CommandCompletion( void(*callback)(const char *s) );
 // callback with each valid string
 void Cmd_SetCommandCompletionFunc( const char *command,
@@ -501,11 +504,15 @@
 void 	Cvar_Set( const char *var_name, const char *value );
 // will create the variable with no flags if it doesn't exist
 
+void	Cvar_SetSafe( const char *var_name, const char *value );
+// sometimes we set variables from an untrusted source: fail if flags & CVAR_PROTECTED
+
 void Cvar_SetLatched( const char *var_name, const char *value);
 // don't set the cvar immediately
 
 void	Cvar_SetValue( const char *var_name, float value );
-// expands value to a string and calls Cvar_Set
+void	Cvar_SetValueSafe( const char *var_name, float value );
+// expands value to a string and calls Cvar_Set/Cvar_SetSafe
 
 float	Cvar_VariableValue( const char *var_name );
 int		Cvar_VariableIntegerValue( const char *var_name );

Modified: trunk/code/server/sv_game.c
===================================================================
--- trunk/code/server/sv_game.c	2011-03-08 01:37:28 UTC (rev 1915)
+++ trunk/code/server/sv_game.c	2011-03-08 01:39:34 UTC (rev 1916)
@@ -314,7 +314,7 @@
 		Cvar_Update( VMA(1) );
 		return 0;
 	case G_CVAR_SET:
-		Cvar_Set( (const char *)VMA(1), (const char *)VMA(2) );
+		Cvar_SetSafe( (const char *)VMA(1), (const char *)VMA(2) );
 		return 0;
 	case G_CVAR_VARIABLE_INTEGER_VALUE:
 		return Cvar_VariableIntegerValue( (const char *)VMA(1) );

Modified: trunk/code/server/sv_main.c
===================================================================
--- trunk/code/server/sv_main.c	2011-03-08 01:37:28 UTC (rev 1915)
+++ trunk/code/server/sv_main.c	2011-03-08 01:39:34 UTC (rev 1916)
@@ -645,12 +645,12 @@
 	Info_SetValueForKey( infostring, "hostname", sv_hostname->string );
 	Info_SetValueForKey( infostring, "mapname", sv_mapname->string );
 	Info_SetValueForKey( infostring, "clients", va("%i", count) );
-	Info_SetValueForKey( infostring, "g_humanplayers", va("%i", humans ) );
+	Info_SetValueForKey(infostring, "g_humanplayers", va("%i", humans));
 	Info_SetValueForKey( infostring, "sv_maxclients", 
 		va("%i", sv_maxclients->integer - sv_privateClients->integer ) );
 	Info_SetValueForKey( infostring, "gametype", va("%i", sv_gametype->integer ) );
 	Info_SetValueForKey( infostring, "pure", va("%i", sv_pure->integer ) );
-	Info_SetValueForKey( infostring, "g_needpass", Cvar_VariableIntegerValue( "g_needpass" ));
+	Info_SetValueForKey(infostring, "g_needpass", va("%d", Cvar_VariableIntegerValue("g_needpass")));
 
 #ifdef USE_VOIP
 	if (sv_voip->integer) {



More information about the quake3-commits mailing list