Skip to content

Commit

Permalink
- Fix potential out-of-bounds read in files.c, fix by using new FS_IsExt
Browse files Browse the repository at this point in the history 
- Add capability to load demos with com_protocol suffix, partially applied patches from Simon McVittie
- Fix demo loading if protocol number has more digits than 2
- Minor refactoring, replace all occurances of suffix "dm_" with global macro DEMOEXT
  • Loading branch information
Thilo Schulz committed Mar 7, 2011
1 parent fef4d12 commit c0cca7a
Show file tree
Hide file tree
Showing 6 changed files with 124 additions and 47 deletions.
68 changes: 44 additions & 24 deletions code/client/cl_main.c
View file
Expand Up @@ -627,14 +627,14 @@ void CL_Record_f( void ) {
if ( Cmd_Argc() == 2 ) {
s = Cmd_Argv(1);
Q_strncpyz( demoName, s, sizeof( demoName ) );
Com_sprintf (name, sizeof(name), "demos/%s.dm_%d", demoName, PROTOCOL_VERSION );
Com_sprintf (name, sizeof(name), "demos/%s.%s%d", demoName, DEMOEXT, com_protocol->integer );
} else {
int number;

// scan for a free demo name
for ( number = 0 ; number <= 9999 ; number++ ) {
CL_DemoFilename( number, demoName );
Com_sprintf (name, sizeof(name), "demos/%s.dm_%d", demoName, PROTOCOL_VERSION );
Com_sprintf (name, sizeof(name), "demos/%s.%s%d", demoName, DEMOEXT, com_protocol->integer );

if (!FS_FileExists(name))
break; // file doesn't exist
Expand Down Expand Up @@ -884,9 +884,22 @@ static void CL_WalkDemoExt(char *arg, char *name, int *demofile)
{
int i = 0;
*demofile = 0;

Com_sprintf (name, MAX_OSPATH, "demos/%s.%s%d", arg, DEMOEXT, com_protocol->integer);

FS_FOpenFileRead( name, demofile, qtrue );

if (*demofile)
{
Com_Printf("Demo file: %s\n", name);
return;
}

Com_Printf("Not found: %s\n", name);

while(demo_protocols[i])
{
Com_sprintf (name, MAX_OSPATH, "demos/%s.dm_%d", arg, demo_protocols[i]);
Com_sprintf (name, MAX_OSPATH, "demos/%s.%s%d", arg, DEMOEXT, demo_protocols[i]);
FS_FOpenFileRead( name, demofile, qtrue );
if (*demofile)
{
Expand All @@ -910,7 +923,7 @@ static void CL_CompleteDemoName( char *args, int argNum )
{
char demoExt[ 16 ];

Com_sprintf( demoExt, sizeof( demoExt ), ".dm_%d", PROTOCOL_VERSION );
Com_sprintf(demoExt, sizeof(demoExt), ".%s%d", DEMOEXT, com_protocol->integer);
Field_CompleteFilename( "demos", demoExt, qtrue, qtrue );
}
}
Expand Down Expand Up @@ -943,34 +956,41 @@ void CL_PlayDemo_f( void ) {

CL_Disconnect( qtrue );

// check for an extension .dm_?? (?? is protocol)
ext_test = arg + strlen(arg) - 6;
if ((strlen(arg) > 6) && (ext_test[0] == '.') &&
((ext_test[1] == 'd') || (ext_test[1] == 'D')) &&
((ext_test[2] == 'm') || (ext_test[2] == 'M')) &&
(ext_test[3] == '_'))
// check for an extension .DEMOEXT_?? (?? is protocol)
ext_test = Q_strrchr(arg, '.');

if(ext_test && !Q_stricmpn(ext_test + 1, DEMOEXT, ARRAY_LEN(DEMOEXT) - 1))
{
protocol = atoi(ext_test+4);
i=0;
while(demo_protocols[i])
protocol = atoi(ext_test + ARRAY_LEN(DEMOEXT));

for(i = 0; demo_protocols[i]; i++)
{
if (demo_protocols[i] == protocol)
if(demo_protocols[i] == protocol)
break;
i++;
}
if (demo_protocols[i])

if(demo_protocols[i] || protocol == com_protocol->integer)
{
Com_sprintf (name, sizeof(name), "demos/%s", arg);
FS_FOpenFileRead( name, &clc.demofile, qtrue );
} else {
Com_sprintf(name, sizeof(name), "demos/%s", arg);
FS_FOpenFileRead(name, &clc.demofile, qtrue);
}
else
{
int len;

Com_Printf("Protocol %d not supported for demos\n", protocol);
Q_strncpyz(retry, arg, sizeof(retry));
retry[strlen(retry)-6] = 0;
CL_WalkDemoExt( retry, name, &clc.demofile );
len = ext_test - arg;

if(len >= ARRAY_LEN(retry))
len = ARRAY_LEN(retry) - 1;

Q_strncpyz(retry, arg, len + 1);
retry[len] = '\0';
CL_WalkDemoExt(retry, name, &clc.demofile);
}
} else {
CL_WalkDemoExt( arg, name, &clc.demofile );
}
else
CL_WalkDemoExt(arg, name, &clc.demofile);

if (!clc.demofile) {
Com_Error( ERR_DROP, "couldn't open %s", name);
Expand Down
2 changes: 1 addition & 1 deletion code/q3_ui/ui_demo2.c
View file
Expand Up @@ -223,7 +223,7 @@ static void Demos_MenuInit( void ) {
s_demos.list.generic.y = 130;
s_demos.list.width = 16;
s_demos.list.height = 14;
Com_sprintf(extension, sizeof(extension), "dm_%d", (int)trap_Cvar_VariableValue( "protocol" ) );
Com_sprintf(extension, sizeof(extension), ".%s%d", DEMOEXT, (int) trap_Cvar_VariableValue("protocol"));
s_demos.list.numitems = trap_FS_GetFileList( "demos", extension, s_demos.names, NAMEBUFSIZE );
s_demos.list.itemnames = (const char **)s_demos.demolist;
s_demos.list.columns = 3;
Expand Down
93 changes: 74 additions & 19 deletions code/qcommon/files.c
View file
Expand Up @@ -941,6 +941,59 @@ qboolean FS_FilenameCompare( const char *s1, const char *s2 ) {
return qfalse; // strings are equal
}

/*
===========
FS_IsExt
Return qtrue if ext matches file extension filename
===========
*/

qboolean FS_IsExt(const char *filename, const char *ext, int namelen)
{
int extlen;

extlen = strlen(ext);

if(extlen > namelen)
return qfalse;

filename += namelen - extlen;

return !Q_stricmp(filename, ext);
}

/*
===========
FS_IsDemoExt
Return qtrue if filename has a demo extension
===========
*/

qboolean FS_IsDemoExt(const char *filename, int namelen)
{
char *ext_test;
int index, protocol;

ext_test = Q_strrchr(filename, '.');
if(ext_test && !Q_stricmpn(ext_test + 1, DEMOEXT, ARRAY_LEN(DEMOEXT) - 1))
{
protocol = atoi(ext_test + ARRAY_LEN(DEMOEXT));

if(protocol == com_protocol->integer)
return qtrue;

for(index = 0; demo_protocols[index]; index++)
{
if(demo_protocols[index] == protocol)
return qtrue;
}
}

return qfalse;
}

/*
===========
FS_FOpenFileRead
Expand All @@ -962,7 +1015,6 @@ int FS_FOpenFileRead( const char *filename, fileHandle_t *file, qboolean uniqueF
long hash;
FILE *temp;
int l;
char demoExt[16];

hash = 0;

Expand Down Expand Up @@ -1009,7 +1061,6 @@ int FS_FOpenFileRead( const char *filename, fileHandle_t *file, qboolean uniqueF
Com_Error( ERR_FATAL, "FS_FOpenFileRead: NULL 'filename' parameter passed\n" );
}

Com_sprintf (demoExt, sizeof(demoExt), ".dm_%d",PROTOCOL_VERSION );
// qpaths are not supposed to have a leading slash
if ( filename[0] == '/' || filename[0] == '\\' ) {
filename++;
Expand Down Expand Up @@ -1061,16 +1112,19 @@ int FS_FOpenFileRead( const char *filename, fileHandle_t *file, qboolean uniqueF
// shaders, txt, arena files by themselves do not count as a reference as
// these are loaded from all pk3s
// from every pk3 file..
l = strlen( filename );
if ( !(pak->referenced & FS_GENERAL_REF)) {
if ( Q_stricmp(filename + l - 7, ".shader") != 0 &&
Q_stricmp(filename + l - 4, ".txt") != 0 &&
Q_stricmp(filename + l - 4, ".cfg") != 0 &&
Q_stricmp(filename + l - 7, ".config") != 0 &&
strstr(filename, "levelshots") == NULL &&
Q_stricmp(filename + l - 4, ".bot") != 0 &&
Q_stricmp(filename + l - 6, ".arena") != 0 &&
Q_stricmp(filename + l - 5, ".menu") != 0) {
l = strlen(filename);

if (!(pak->referenced & FS_GENERAL_REF))
{
if(!FS_IsExt(filename, ".shader", l) &&
!FS_IsExt(filename, ".txt", l) &&
!FS_IsExt(filename, ".cfg", l) &&
!FS_IsExt(filename, ".config", l) &&
!FS_IsExt(filename, ".bot", l) &&
!FS_IsExt(filename, ".arena", l) &&
!FS_IsExt(filename, ".menu", l) &&
!strstr(filename, "levelshots"))
{
pak->referenced |= FS_GENERAL_REF;
}
}
Expand Down Expand Up @@ -1121,13 +1175,14 @@ int FS_FOpenFileRead( const char *filename, fileHandle_t *file, qboolean uniqueF
// this test can make the search fail although the file is in the directory
// I had the problem on https://zerowing.idsoftware.com/bugzilla/show_bug.cgi?id=8
// turned out I used FS_FileExists instead
if ( fs_numServerPaks ) {

if ( Q_stricmp( filename + l - 4, ".cfg" ) // for config files
&& Q_stricmp( filename + l - 5, ".menu" ) // menu files
&& Q_stricmp( filename + l - 5, ".game" ) // menu files
&& Q_stricmp( filename + l - strlen(demoExt), demoExt ) // menu files
&& Q_stricmp( filename + l - 4, ".dat" ) ) { // for journal files
if(fs_numServerPaks)
{
if(!FS_IsExt(filename, ".cfg", l) && // for config files
!FS_IsExt(filename, ".menu", l) && // menu files
!FS_IsExt(filename, ".game", l) && // menu files
!FS_IsExt(filename, ".cfg", l) && // for journal files
!FS_IsDemoExt(filename, l)) // demos
{
continue;
}
}
Expand Down
2 changes: 2 additions & 0 deletions code/qcommon/q_shared.h
View file
Expand Up @@ -55,6 +55,8 @@ Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
#define MAX_TEAMNAME 32
#define MAX_MASTER_SERVERS 5 // number of supported master servers

#define DEMOEXT "dm_" // standard demo extension

#ifdef _MSC_VER

#pragma warning(disable : 4018) // signed/unsigned mismatch
Expand Down
2 changes: 1 addition & 1 deletion code/ui/ui_atoms.c
View file
Expand Up @@ -162,7 +162,7 @@ void UI_LoadBestScores(const char *map, int game) {
}
UI_SetBestScores(&newInfo, qfalse);

Com_sprintf(fileName, MAX_QPATH, "demos/%s_%d.dm_%d", map, game, (int)trap_Cvar_VariableValue("protocol"));
Com_sprintf(fileName, MAX_QPATH, "demos/%s_%d.%s%d", map, game, DEMOEXT, (int)trap_Cvar_VariableValue("protocol"));
uiInfo.demoAvailable = qfalse;
if (trap_FS_FOpenFile(fileName, &f, FS_READ) >= 0) {
uiInfo.demoAvailable = qtrue;
Expand Down
4 changes: 2 additions & 2 deletions code/ui/ui_main.c
View file
Expand Up @@ -2877,11 +2877,11 @@ static void UI_LoadDemos( void ) {
char *demoname;
int i, len;

Com_sprintf(demoExt, sizeof(demoExt), "dm_%d", (int)trap_Cvar_VariableValue("protocol"));
Com_sprintf(demoExt, sizeof(demoExt), "%s%d", DEMOEXT, (int)trap_Cvar_VariableValue("protocol"));

uiInfo.demoCount = trap_FS_GetFileList( "demos", demoExt, demolist, 4096 );

Com_sprintf(demoExt, sizeof(demoExt), ".dm_%d", (int)trap_Cvar_VariableValue("protocol"));
Com_sprintf(demoExt, sizeof(demoExt), ".%s%d", DEMOEXT, (int)trap_Cvar_VariableValue("protocol"));

if (uiInfo.demoCount) {
if (uiInfo.demoCount > MAX_DEMOS) {
Expand Down

0 comments on commit c0cca7a

Please sign in to comment.