Fix fs_game '..' reading outside of home and base path

VMs could set fs_game to '..' at anytime to access files outside of home
and base path. fs_game sent by server to clients could also be '..' to
access files outside of home and base path.

'..' was not caught by FS_CheckDirTraversal() as it expects filenames
not a single directory.

I've made fs_game be latched to prevent VMs from changing it with no
good way to validate it before it's used. com_basegame and fs_basegame
are now latched as well.

Additionally, it's now possible to change com_basegame while the engine
is running. game_restart or vid_restart will make it take affect.
com_homepath is now CVAR_PROTECTED to prevent VMs from changing it
to a directory traversal.

This requires my two previous commits for preventing VMs from changing
engine latch cvars and only Cvar_Get fs_game in FS_Startup (so CVAR_INIT
isn't added in serveral other places).

Reported by Noah Metzger (Chomenor).
This commit is contained in:
Zack Middleton 2018-01-21 04:27:55 -06:00
parent 78ca670d4f
commit 3638f69dff
5 changed files with 54 additions and 32 deletions

View file

@ -1245,7 +1245,7 @@ void CL_ClearMemory(qboolean shutdownRef)
CL_ShutdownAll(shutdownRef); CL_ShutdownAll(shutdownRef);
// if not running a server clear the whole hunk // if not running a server clear the whole hunk
if ( !com_sv_running->integer ) { if ( !com_sv_running || !com_sv_running->integer ) {
// clear the whole hunk // clear the whole hunk
Hunk_Clear(); Hunk_Clear();
// clear collision map data // clear collision map data
@ -1361,7 +1361,7 @@ static void CL_OldGame(void)
{ {
// change back to previous fs_game // change back to previous fs_game
cl_oldGameSet = qfalse; cl_oldGameSet = qfalse;
Cvar_Set2("fs_game", cl_oldGame, qtrue); Cvar_Set("fs_game", cl_oldGame);
FS_ConditionalRestart(clc.checksumFeed, qfalse); FS_ConditionalRestart(clc.checksumFeed, qfalse);
} }
} }

View file

@ -399,7 +399,7 @@ void CL_SystemInfoChanged( void ) {
// ehw! // ehw!
if (!Q_stricmp(key, "fs_game")) if (!Q_stricmp(key, "fs_game"))
{ {
if(FS_CheckDirTraversal(value)) if(FS_InvalidGameDir(value))
{ {
Com_Printf(S_COLOR_YELLOW "WARNING: Server sent invalid fs_game value %s\n", value); Com_Printf(S_COLOR_YELLOW "WARNING: Server sent invalid fs_game value %s\n", value);
continue; continue;

View file

@ -2404,6 +2404,9 @@ void Com_GameRestart(int checksumFeed, qboolean disconnect)
CL_Shutdown("Game directory changed", disconnect, qfalse); CL_Shutdown("Game directory changed", disconnect, qfalse);
} }
// change com_basegame to latched value
com_basegame = Cvar_Get("com_basegame", BASEGAME, CVAR_LATCH|CVAR_NORESTART);
FS_Restart(checksumFeed); FS_Restart(checksumFeed);
// Clean out any user and VM created cvars // Clean out any user and VM created cvars
@ -2439,16 +2442,7 @@ Expose possibility to change current running mod to the user
void Com_GameRestart_f(void) void Com_GameRestart_f(void)
{ {
if(!FS_FilenameCompare(Cmd_Argv(1), com_basegame->string)) Cvar_Set("fs_game", Cmd_Argv(1));
{
// This is the standard base game. Servers and clients should
// use "" and not the standard basegame name because this messes
// up pak file negotiation and lots of other stuff
Cvar_Set("fs_game", "");
}
else
Cvar_Set("fs_game", Cmd_Argv(1));
Com_GameRestart(0, qtrue); Com_GameRestart(0, qtrue);
} }
@ -2705,11 +2699,8 @@ void Com_Init( char *commandLine ) {
CL_InitKeyCommands(); CL_InitKeyCommands();
com_standalone = Cvar_Get("com_standalone", "0", CVAR_ROM); com_standalone = Cvar_Get("com_standalone", "0", CVAR_ROM);
com_basegame = Cvar_Get("com_basegame", BASEGAME, CVAR_INIT); com_basegame = Cvar_Get("com_basegame", BASEGAME, CVAR_LATCH|CVAR_NORESTART);
com_homepath = Cvar_Get("com_homepath", "", CVAR_INIT); com_homepath = Cvar_Get("com_homepath", "", CVAR_INIT|CVAR_PROTECTED);
if(!com_basegame->string[0])
Cvar_ForceReset("com_basegame");
FS_InitFilesystem (); FS_InitFilesystem ();

View file

@ -3067,6 +3067,24 @@ qboolean FS_CheckDirTraversal(const char *checkdir)
return qfalse; return qfalse;
} }
/*
================
FS_InvalidGameDir
return true if path is a reference to current directory or directory traversal
================
*/
qboolean FS_InvalidGameDir( const char *gamedir ) {
if ( !strcmp( gamedir, "." ) || !strcmp( gamedir, ".." )
|| !strcmp( gamedir, "/" ) || !strcmp( gamedir, "\\" )
|| strstr( gamedir, "/.." ) || strstr( gamedir, "\\.." )
|| FS_CheckDirTraversal( gamedir ) ) {
return qtrue;
}
return qfalse;
}
/* /*
================ ================
FS_ComparePaks FS_ComparePaks
@ -3301,13 +3319,34 @@ static void FS_Startup( const char *gameName )
fs_debug = Cvar_Get( "fs_debug", "0", 0 ); fs_debug = Cvar_Get( "fs_debug", "0", 0 );
fs_basepath = Cvar_Get ("fs_basepath", Sys_DefaultInstallPath(), CVAR_INIT|CVAR_PROTECTED ); fs_basepath = Cvar_Get ("fs_basepath", Sys_DefaultInstallPath(), CVAR_INIT|CVAR_PROTECTED );
fs_basegame = Cvar_Get ("fs_basegame", "", CVAR_INIT ); fs_basegame = Cvar_Get ("fs_basegame", "", CVAR_LATCH|CVAR_NORESTART );
homePath = Sys_DefaultHomePath(); homePath = Sys_DefaultHomePath();
if (!homePath || !homePath[0]) { if (!homePath || !homePath[0]) {
homePath = fs_basepath->string; homePath = fs_basepath->string;
} }
fs_homepath = Cvar_Get ("fs_homepath", homePath, CVAR_INIT|CVAR_PROTECTED ); fs_homepath = Cvar_Get ("fs_homepath", homePath, CVAR_INIT|CVAR_PROTECTED );
fs_gamedirvar = Cvar_Get ("fs_game", "", CVAR_INIT|CVAR_SYSTEMINFO ); fs_gamedirvar = Cvar_Get ("fs_game", "", CVAR_LATCH|CVAR_NORESTART|CVAR_SYSTEMINFO );
if (!gameName[0]) {
Cvar_ForceReset( "com_basegame" );
}
if (!FS_FilenameCompare(fs_gamedirvar->string, gameName)) {
// This is the standard base game. Servers and clients should
// use "" and not the standard basegame name because this messes
// up pak file negotiation and lots of other stuff
Cvar_ForceReset( "fs_game" );
}
if (FS_InvalidGameDir(gameName)) {
Com_Error( ERR_DROP, "Invalid com_basegame '%s'", gameName );
}
if (FS_InvalidGameDir(fs_basegame->string)) {
Com_Error( ERR_DROP, "Invalid fs_basegame '%s'", fs_basegame->string );
}
if (FS_InvalidGameDir(fs_gamedirvar->string)) {
Com_Error( ERR_DROP, "Invalid fs_game '%s'", fs_gamedirvar->string );
}
// add search path elements in reverse priority order // add search path elements in reverse priority order
fs_gogpath = Cvar_Get ("fs_gogpath", Sys_GogPath(), CVAR_INIT|CVAR_PROTECTED ); fs_gogpath = Cvar_Get ("fs_gogpath", Sys_GogPath(), CVAR_INIT|CVAR_PROTECTED );
@ -3391,8 +3430,6 @@ static void FS_Startup( const char *gameName )
// print the current search paths // print the current search paths
FS_Path_f(); FS_Path_f();
fs_gamedirvar->modified = qfalse; // We just loaded, it's not modified
Com_Printf( "----------------------\n" ); Com_Printf( "----------------------\n" );
#ifdef FS_MISSING #ifdef FS_MISSING
@ -4040,17 +4077,10 @@ Return qtrue if restarting due to game directory changed, qfalse otherwise
*/ */
qboolean FS_ConditionalRestart(int checksumFeed, qboolean disconnect) qboolean FS_ConditionalRestart(int checksumFeed, qboolean disconnect)
{ {
if(fs_gamedirvar->modified) if (com_basegame->latchedString || fs_basegame->latchedString || fs_gamedirvar->latchedString)
{ {
if(FS_FilenameCompare(lastValidGame, fs_gamedirvar->string) && Com_GameRestart(checksumFeed, disconnect);
(*lastValidGame || FS_FilenameCompare(fs_gamedirvar->string, com_basegame->string)) && return qtrue;
(*fs_gamedirvar->string || FS_FilenameCompare(lastValidGame, com_basegame->string)))
{
Com_GameRestart(checksumFeed, disconnect);
return qtrue;
}
else
fs_gamedirvar->modified = qfalse;
} }
if(checksumFeed != fs_checksumFeed) if(checksumFeed != fs_checksumFeed)

View file

@ -726,6 +726,7 @@ void FS_PureServerSetLoadedPaks( const char *pakSums, const char *pakNames );
// sole exception of .cfg files. // sole exception of .cfg files.
qboolean FS_CheckDirTraversal(const char *checkdir); qboolean FS_CheckDirTraversal(const char *checkdir);
qboolean FS_InvalidGameDir(const char *gamedir);
qboolean FS_idPak(char *pak, char *base, int numPaks); qboolean FS_idPak(char *pak, char *base, int numPaks);
qboolean FS_ComparePaks( char *neededpaks, int len, qboolean dlstring ); qboolean FS_ComparePaks( char *neededpaks, int len, qboolean dlstring );