From 8550620849ffe8c1ec8c706932659a40646cb6b7 Mon Sep 17 00:00:00 2001 From: Jonathan Gray Date: Tue, 7 May 2013 15:50:26 +1000 Subject: [PATCH] CVE-2006-3325 arbitrary cvar overwrite CVE-2006-3325 client/cl_parse.c in the id3 Quake 3 Engine 1.32c and the Icculus Quake 3 Engine (ioquake3) revision 810 and earlier allows remote malicious servers to overwrite arbitrary write-protected cvars variables on the client, such as cl_allowdownload for Automatic Downloading and fs_homepath for the quake3 path, via a string of cvar names and values sent from the server. NOTE: this can be combined with another vulnerability to overwrite arbitrary files. Luigi Auriemma q3cfilevar from Thilo Schulz in ioquake3 svn 811 git 7d51d75b05a9593508040162709043516c0f2a17 - Fix arbitrary cvar overwrite flaw: http://aluigi.altervista.org/adv.htm --- codemp/client/cl_parse.cpp | 24 ++++++++++++++++++++++-- codemp/game/q_shared.h | 2 ++ codemp/qcommon/cvar.cpp | 14 ++++++++++++++ codemp/qcommon/files_pc.cpp | 17 +++++++++++++++++ codemp/qcommon/qcommon.h | 4 ++++ 5 files changed, 59 insertions(+), 2 deletions(-) diff --git a/codemp/client/cl_parse.cpp b/codemp/client/cl_parse.cpp index d41ccf4..d267eb0 100644 --- a/codemp/client/cl_parse.cpp +++ b/codemp/client/cl_parse.cpp @@ -432,19 +432,39 @@ 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(S_COLOR_YELLOW "WARNING: Server sent invalid fs_game value %s\n", value); + continue; + } + gameSet = qtrue; } + 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); + } 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") ) { + if (!gameSet && *Cvar_VariableString("fs_game") ) { Cvar_Set( "fs_game", "" ); } cl_connectedToPureServer = Cvar_VariableValue( "sv_pure" ); diff --git a/codemp/game/q_shared.h b/codemp/game/q_shared.h index 5264fb6..4769278 100644 --- a/codemp/game/q_shared.h +++ b/codemp/game/q_shared.h @@ -1860,6 +1860,8 @@ default values. #define CVAR_NORESTART 0x00000400 // do not clear when a cvar_restart is issued #define CVAR_INTERNAL 0x00000800 // cvar won't be displayed, ever (for passwords and such) #define CVAR_PARENTAL 0x00001000 // lets cvar system know that parental stuff needs to be updated +#define CVAR_SERVER_CREATED 0x00002000 // 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 { diff --git a/codemp/qcommon/cvar.cpp b/codemp/qcommon/cvar.cpp index 03a0282..17f1491 100644 --- a/codemp/qcommon/cvar.cpp +++ b/codemp/qcommon/cvar.cpp @@ -157,6 +157,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/codemp/qcommon/files_pc.cpp b/codemp/qcommon/files_pc.cpp index 261d611..d8e80d9 100644 --- a/codemp/qcommon/files_pc.cpp +++ b/codemp/qcommon/files_pc.cpp @@ -2312,6 +2312,23 @@ qboolean FS_idPak( char *pak, char *base ) { return qfalse; } +/* +================ +FS_CheckDirTraversal + +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 diff --git a/codemp/qcommon/qcommon.h b/codemp/qcommon/qcommon.h index 3fb25d0..8c0adaa 100644 --- a/codemp/qcommon/qcommon.h +++ b/codemp/qcommon/qcommon.h @@ -452,6 +452,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 @@ -623,6 +626,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 ); void FS_Rename( const char *from, const char *to );