From d21411452ef32b86c0b79ddcaf49221701dcdb07 Mon Sep 17 00:00:00 2001 From: Thilo Schulz Date: Sat, 6 May 2006 01:56:24 +0000 Subject: [PATCH] Add string length checking to function COM_StripExtension. This fixes the R_RemapShader buffer overflow exploit that can be found here: http://milw0rm.com/exploits/1750 --- code/cgame/cg_weapons.c | 6 +++--- code/client/cl_main.c | 2 +- code/q3_ui/ui_playermodel.c | 4 ++-- code/q3_ui/ui_players.c | 4 ++-- code/q3_ui/ui_saveconfig.c | 2 +- code/qcommon/files.c | 2 +- code/qcommon/q_shared.c | 4 ++-- code/qcommon/q_shared.h | 2 +- code/qcommon/vm.c | 2 +- code/renderer/tr_bsp.c | 2 +- code/renderer/tr_shader.c | 6 +++--- code/ui/ui_main.c | 4 ++-- code/ui/ui_players.c | 4 ++-- 13 files changed, 22 insertions(+), 22 deletions(-) diff --git a/code/cgame/cg_weapons.c b/code/cgame/cg_weapons.c index d15de2dc..49e2697a 100644 --- a/code/cgame/cg_weapons.c +++ b/code/cgame/cg_weapons.c @@ -656,17 +656,17 @@ void CG_RegisterWeapon( int weaponNum ) { } strcpy( path, item->world_model[0] ); - COM_StripExtension( path, path ); + COM_StripExtension(path, path, sizeof(path)); strcat( path, "_flash.md3" ); weaponInfo->flashModel = trap_R_RegisterModel( path ); strcpy( path, item->world_model[0] ); - COM_StripExtension( path, path ); + COM_StripExtension(path, path, sizeof(path)); strcat( path, "_barrel.md3" ); weaponInfo->barrelModel = trap_R_RegisterModel( path ); strcpy( path, item->world_model[0] ); - COM_StripExtension( path, path ); + COM_StripExtension(path, path, sizeof(path)); strcat( path, "_hand.md3" ); weaponInfo->handsModel = trap_R_RegisterModel( path ); diff --git a/code/client/cl_main.c b/code/client/cl_main.c index 7b207e5e..9a1bcf68 100644 --- a/code/client/cl_main.c +++ b/code/client/cl_main.c @@ -2066,7 +2066,7 @@ void CL_Frame ( int msec ) { } Q_strncpyz( mapName, COM_SkipPath( cl.mapname ), sizeof( cl.mapname ) ); - COM_StripExtension( mapName, mapName ); + COM_StripExtension(mapName, mapName, sizeof(mapName)); Cbuf_ExecuteText( EXEC_NOW, va( "record %s-%s-%s", nowString, serverName, mapName ) ); diff --git a/code/q3_ui/ui_playermodel.c b/code/q3_ui/ui_playermodel.c index 009f77d2..e2471498 100644 --- a/code/q3_ui/ui_playermodel.c +++ b/code/q3_ui/ui_playermodel.c @@ -391,7 +391,7 @@ static void PlayerModel_BuildList( void ) int numfiles; char dirlist[2048]; char filelist[2048]; - char skinname[64]; + char skinname[MAX_QPATH]; char* dirptr; char* fileptr; int i; @@ -424,7 +424,7 @@ static void PlayerModel_BuildList( void ) { filelen = strlen(fileptr); - COM_StripExtension(fileptr,skinname); + COM_StripExtension(fileptr,skinname, sizeof(skinname)); // look for icon_???? if (!Q_stricmpn(skinname,"icon_",5)) diff --git a/code/q3_ui/ui_players.c b/code/q3_ui/ui_players.c index db4b4385..182b5f0d 100644 --- a/code/q3_ui/ui_players.c +++ b/code/q3_ui/ui_players.c @@ -89,13 +89,13 @@ tryagain: if ( weaponNum == WP_MACHINEGUN || weaponNum == WP_GAUNTLET || weaponNum == WP_BFG ) { strcpy( path, item->world_model[0] ); - COM_StripExtension( path, path ); + COM_StripExtension( path, path, sizeof(path) ); strcat( path, "_barrel.md3" ); pi->barrelModel = trap_R_RegisterModel( path ); } strcpy( path, item->world_model[0] ); - COM_StripExtension( path, path ); + COM_StripExtension( path, path, sizeof(path) ); strcat( path, "_flash.md3" ); pi->flashModel = trap_R_RegisterModel( path ); diff --git a/code/q3_ui/ui_saveconfig.c b/code/q3_ui/ui_saveconfig.c index e9884639..c9f0bc25 100644 --- a/code/q3_ui/ui_saveconfig.c +++ b/code/q3_ui/ui_saveconfig.c @@ -85,7 +85,7 @@ static void UI_SaveConfigMenu_SaveEvent( void *ptr, int event ) { return; } - COM_StripExtension(saveConfig.savename.field.buffer, configname ); + COM_StripExtension(saveConfig.savename.field.buffer, configname, sizeof(configname)); trap_Cmd_ExecuteText( EXEC_APPEND, va( "writeconfig %s.cfg\n", configname ) ); UI_PopMenu(); } diff --git a/code/qcommon/files.c b/code/qcommon/files.c index 51bf5efc..13b8a257 100644 --- a/code/qcommon/files.c +++ b/code/qcommon/files.c @@ -3451,7 +3451,7 @@ void FS_FilenameCompletion( const char *dir, const char *ext, Q_strncpyz( filename, filenames[ i ], MAX_STRING_CHARS ); if( stripExt ) { - COM_StripExtension( filename, filename ); + COM_StripExtension(filename, filename, sizeof(filename)); } callback( filename ); diff --git a/code/qcommon/q_shared.c b/code/qcommon/q_shared.c index de54e179..9d94f2a6 100644 --- a/code/qcommon/q_shared.c +++ b/code/qcommon/q_shared.c @@ -58,10 +58,10 @@ char *COM_SkipPath (char *pathname) COM_StripExtension ============ */ -void COM_StripExtension( const char *in, char *out ) { +void COM_StripExtension( const char *in, char *out, int destsize ) { int length; - strcpy( out, in ); + Q_strncpyz(out, in, destsize); length = strlen(out)-1; while (length > 0 && out[length] != '.') diff --git a/code/qcommon/q_shared.h b/code/qcommon/q_shared.h index 3fc713d8..705517a7 100644 --- a/code/qcommon/q_shared.h +++ b/code/qcommon/q_shared.h @@ -588,7 +588,7 @@ int Q_isnan( float x ); float Com_Clamp( float min, float max, float value ); char *COM_SkipPath( char *pathname ); -void COM_StripExtension( const char *in, char *out ); +void COM_StripExtension(const char *in, char *out, int destsize); void COM_DefaultExtension( char *path, int maxSize, const char *extension ); void COM_BeginParseSession( const char *name ); diff --git a/code/qcommon/vm.c b/code/qcommon/vm.c index 0eb35c54..efc0da83 100644 --- a/code/qcommon/vm.c +++ b/code/qcommon/vm.c @@ -230,7 +230,7 @@ void VM_LoadSymbols( vm_t *vm ) { return; } - COM_StripExtension( vm->name, name ); + COM_StripExtension(vm->name, name, sizeof(name)); Com_sprintf( symbols, sizeof( symbols ), "vm/%s.map", name ); len = FS_ReadFile( symbols, (void **)&mapfile ); if ( !mapfile ) { diff --git a/code/renderer/tr_bsp.c b/code/renderer/tr_bsp.c index 85909baa..c340e134 100644 --- a/code/renderer/tr_bsp.c +++ b/code/renderer/tr_bsp.c @@ -1823,7 +1823,7 @@ void RE_LoadWorldMap( const char *name ) { Q_strncpyz( s_worldData.name, name, sizeof( s_worldData.name ) ); Q_strncpyz( s_worldData.baseName, COM_SkipPath( s_worldData.name ), sizeof( s_worldData.name ) ); - COM_StripExtension( s_worldData.baseName, s_worldData.baseName ); + COM_StripExtension(s_worldData.baseName, s_worldData.baseName, sizeof(s_worldData.baseName)); startMarker = ri.Hunk_Alloc(0, h_low); c_gridVerts = 0; diff --git a/code/renderer/tr_shader.c b/code/renderer/tr_shader.c index dc5b7d70..dac45ee4 100644 --- a/code/renderer/tr_shader.c +++ b/code/renderer/tr_shader.c @@ -95,7 +95,7 @@ void R_RemapShader(const char *shaderName, const char *newShaderName, const char // remap all the shaders with the given name // even tho they might have different lightmaps - COM_StripExtension( shaderName, strippedName ); + COM_StripExtension(shaderName, strippedName, sizeof(strippedName)); hash = generateHashValue(strippedName, FILE_HASH_SIZE); for (sh = hashTable[hash]; sh; sh = sh->next) { if (Q_stricmp(sh->name, strippedName) == 0) { @@ -2365,7 +2365,7 @@ shader_t *R_FindShaderByName( const char *name ) { return tr.defaultShader; } - COM_StripExtension( name, strippedName ); + COM_StripExtension(name, strippedName, sizeof(strippedName)); hash = generateHashValue(strippedName, FILE_HASH_SIZE); @@ -2433,7 +2433,7 @@ shader_t *R_FindShader( const char *name, int lightmapIndex, qboolean mipRawImag lightmapIndex = LIGHTMAP_BY_VERTEX; } - COM_StripExtension( name, strippedName ); + COM_StripExtension(name, strippedName, sizeof(strippedName)); hash = generateHashValue(strippedName, FILE_HASH_SIZE); diff --git a/code/ui/ui_main.c b/code/ui/ui_main.c index c1af0ed7..95d2c347 100644 --- a/code/ui/ui_main.c +++ b/code/ui/ui_main.c @@ -4958,7 +4958,7 @@ static void UI_BuildQ3Model_List( void ) int numfiles; char dirlist[2048]; char filelist[2048]; - char skinname[64]; + char skinname[MAX_QPATH]; char scratch[256]; char* dirptr; char* fileptr; @@ -4988,7 +4988,7 @@ static void UI_BuildQ3Model_List( void ) { filelen = strlen(fileptr); - COM_StripExtension(fileptr,skinname); + COM_StripExtension(fileptr, skinname, sizeof(skinname)); // look for icon_???? if (Q_stricmpn(skinname, "icon_", 5) == 0 && !(Q_stricmp(skinname,"icon_blue") == 0 || Q_stricmp(skinname,"icon_red") == 0)) diff --git a/code/ui/ui_players.c b/code/ui/ui_players.c index c1643276..b09fb320 100644 --- a/code/ui/ui_players.c +++ b/code/ui/ui_players.c @@ -90,13 +90,13 @@ tryagain: if ( weaponNum == WP_MACHINEGUN || weaponNum == WP_GAUNTLET || weaponNum == WP_BFG ) { strcpy( path, item->world_model[0] ); - COM_StripExtension( path, path ); + COM_StripExtension(path, path, sizeof(path)); strcat( path, "_barrel.md3" ); pi->barrelModel = trap_R_RegisterModel( path ); } strcpy( path, item->world_model[0] ); - COM_StripExtension( path, path ); + COM_StripExtension(path, path, sizeof(path)); strcat( path, "_flash.md3" ); pi->flashModel = trap_R_RegisterModel( path );