From: Simon McVittie Date: Fri, 18 Nov 2011 21:03:07 +0000 Subject: CVE-2006-3325: fix arbitrary cvar overwriting Original patch by Thilo Schulz, ioquake3 r811. Bug-Debian: http://bugs.debian.org/660834 Bug-CVE: http://security-tracker.debian.org/tracker/CVE-2006-3325 Origin: backport --- src/client/cl_parse.c | 23 +++++++++++++++++++++-- src/qcommon/cvar.c | 14 ++++++++++++++ src/qcommon/files.c | 19 ++++++++++++++++++- src/qcommon/q_shared.h | 3 +++ src/qcommon/qcommon.h | 4 ++++ 5 files changed, 60 insertions(+), 3 deletions(-) diff --git a/src/client/cl_parse.c b/src/client/cl_parse.c index dc14cd6..2d36aa1 100644 --- src/client/cl_parse.c +++ src/client/cl_parse.c @@ -369,16 +369,35 @@ void CL_SystemInfoChanged( void ) { // scan through all the variables in the systeminfo and locally set cvars to match s = systemInfo; while ( s ) { + int cvar_flags; + Info_NextPair( &s, key, value ); if ( !key[0] ) { break; } + // ehw! - if ( !Q_stricmp( key, "fs_game" ) ) { + if (!Q_stricmp(key, "fs_game")) + { + if(FS_CheckDirTraversal(value)) + { + Com_Printf("WARNING: Server sent invalid fs_game value %s\n", value); + continue; + } + gameSet = qtrue; } - Cvar_Set( key, value ); + if((cvar_flags = Cvar_Flags(key)) == CVAR_NONEXISTENT) + Cvar_Get(key, value, CVAR_SERVER_CREATED | CVAR_ROM); + else + { + // If this cvar may not be modified by a server discard the value. + if(!(cvar_flags & (CVAR_SYSTEMINFO | CVAR_SERVER_CREATED))) + continue; + + Cvar_Set(key, value); + } } // if game folder should not be set and it is set at the client side if ( !gameSet && *Cvar_VariableString("fs_game") ) { diff --git a/src/qcommon/cvar.c b/src/qcommon/cvar.c index a306d88..f6caea8 100644 --- src/qcommon/cvar.c +++ src/qcommon/cvar.c @@ -162,6 +162,20 @@ void Cvar_VariableStringBuffer( const char *var_name, char *buffer, int bufsize } } +/* +============ +Cvar_Flags +============ +*/ +int Cvar_Flags(const char *var_name) +{ + cvar_t *var; + + if(! (var = Cvar_FindVar(var_name)) ) + return CVAR_NONEXISTENT; + else + return var->flags; +} /* ============ diff --git a/src/qcommon/files.c b/src/qcommon/files.c index 34afb4b..49390a0 100644 --- src/qcommon/files.c +++ src/qcommon/files.c @@ -2564,6 +2564,23 @@ qboolean FS_idPak( char *pak, char *base ) { /* ================ +FS_idPak + +Check whether the string contains stuff like "../" to prevent directory traversal bugs +and return qtrue if it does. +================ +*/ + +qboolean FS_CheckDirTraversal(const char *checkdir) +{ + if(strstr(checkdir, "../") || strstr(checkdir, "..\\")) + return qtrue; + + return qfalse; +} + +/* +================ FS_ComparePaks ---------------- @@ -2611,7 +2628,7 @@ qboolean FS_ComparePaks( char *neededpaks, int len, qboolean dlstring ) { } // Make sure the server cannot make us write to non-quake3 directories. - if(strstr(fs_serverReferencedPakNames[i], "../") || strstr(fs_serverReferencedPakNames[i], "..\\")) + if(FS_CheckDirTraversal(fs_serverReferencedPakNames[i])) { Com_Printf("WARNING: Invalid download name %s\n", fs_serverReferencedPakNames[i]); continue; diff --git a/src/qcommon/q_shared.h b/src/qcommon/q_shared.h index 03b2e03..d491708 100644 --- src/qcommon/q_shared.h +++ src/qcommon/q_shared.h @@ -795,6 +795,9 @@ default values. #define CVAR_CHEAT 512 // can not be changed if cheats are disabled #define CVAR_NORESTART 1024 // do not clear when a cvar_restart is issued +#define CVAR_SERVER_CREATED 2048 // cvar was created by a server the client connected to. +#define CVAR_NONEXISTENT 0xFFFFFFFF // Cvar doesn't exist. + // nothing outside the Cvar_*() functions should modify these fields! typedef struct cvar_s { char *name; diff --git a/src/qcommon/qcommon.h b/src/qcommon/qcommon.h index e6ae058..7b2fb8a 100644 --- src/qcommon/qcommon.h +++ src/qcommon/qcommon.h @@ -477,6 +477,9 @@ char *Cvar_VariableString( const char *var_name ); void Cvar_VariableStringBuffer( const char *var_name, char *buffer, int bufsize ); // returns an empty string if not defined +int Cvar_Flags(const char *var_name); +// returns CVAR_NONEXISTENT if cvar doesn't exist or the flags of that particular CVAR. + void Cvar_CommandCompletion( void(*callback)(const char *s) ); // callback with each valid string @@ -647,6 +650,7 @@ void FS_PureServerSetLoadedPaks( const char *pakSums, const char *pakNames ); // separated checksums will be checked for files, with the // sole exception of .cfg files. +qboolean FS_CheckDirTraversal(const char *checkdir); qboolean FS_idPak( char *pak, char *base ); qboolean FS_ComparePaks( char *neededpaks, int len, qboolean dlstring );