Revert my recent cvar latch changes

My cvar latch system changes prevent the Game VM from changing
g_gametype when the value is out of range due to it being registed in
the engine. It's been pointed out as fragile method of security, which
was still exploitable, by Noah Metzger (Chomenor). It doesn't seem like
this is working out to be a good solution.

The issue of fs_game '..' on server being relicated on client via
systeminfo exploit is still fixed as it's not affected by latch.
There are a few cases from current values of fs_game are used which
ideally should use fs_gamedir char array which has been validated.

Revert "Don't let VMs change engine latch cvars immediately"
Partially revert "Fix fs_game '..' reading outside of home and base path"
Revert "Fix VMs forcing engine latch cvar to update to latched value"
This commit is contained in:
Zack Middleton 2018-01-21 22:09:42 -06:00
parent ed8d48cac3
commit 738465d677
4 changed files with 26 additions and 37 deletions

View file

@ -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_Set("fs_game", cl_oldGame); Cvar_Set2("fs_game", cl_oldGame, qtrue);
FS_ConditionalRestart(clc.checksumFeed, qfalse); FS_ConditionalRestart(clc.checksumFeed, qfalse);
} }
} }

View file

@ -2404,9 +2404,6 @@ 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
@ -2699,7 +2696,7 @@ 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_LATCH|CVAR_NORESTART); com_basegame = Cvar_Get("com_basegame", BASEGAME, CVAR_INIT);
com_homepath = Cvar_Get("com_homepath", "", CVAR_INIT|CVAR_PROTECTED); com_homepath = Cvar_Get("com_homepath", "", CVAR_INIT|CVAR_PROTECTED);
FS_InitFilesystem (); FS_InitFilesystem ();

View file

@ -638,29 +638,18 @@ Cvar_SetSafe
void Cvar_SetSafe( const char *var_name, const char *value ) void Cvar_SetSafe( const char *var_name, const char *value )
{ {
int flags = Cvar_Flags( var_name ); int flags = Cvar_Flags( var_name );
qboolean force = qtrue;
if ( flags != CVAR_NONEXISTENT ) if((flags != CVAR_NONEXISTENT) && (flags & CVAR_PROTECTED))
{ {
if ( flags & CVAR_PROTECTED ) if( value )
{ Com_Error( ERR_DROP, "Restricted source tried to set "
if( value ) "\"%s\" to \"%s\"", var_name, value );
Com_Error( ERR_DROP, "Restricted source tried to set " else
"\"%s\" to \"%s\"", var_name, value ); Com_Error( ERR_DROP, "Restricted source tried to "
else "modify \"%s\"", var_name );
Com_Error( ERR_DROP, "Restricted source tried to " return;
"modify \"%s\"", var_name );
return;
}
// don't let VMs or server change engine latched cvars instantly
if ( ( flags & CVAR_LATCH ) && !( flags & CVAR_VM_CREATED ) )
{
force = qfalse;
}
} }
Cvar_Set( var_name, value );
Cvar_Set2 (var_name, value, force);
} }
/* /*
@ -1376,13 +1365,7 @@ void Cvar_Register(vmCvar_t *vmCvar, const char *varName, const char *defaultVal
if ( cv && ( cv->flags & CVAR_PROTECTED ) ) { if ( cv && ( cv->flags & CVAR_PROTECTED ) ) {
Com_DPrintf( S_COLOR_YELLOW "WARNING: VM tried to register protected cvar '%s' with value '%s'%s\n", Com_DPrintf( S_COLOR_YELLOW "WARNING: VM tried to register protected cvar '%s' with value '%s'%s\n",
varName, defaultValue, ( flags & ~cv->flags ) != 0 ? " and new flags" : "" ); varName, defaultValue, ( flags & ~cv->flags ) != 0 ? " and new flags" : "" );
} } else {
// Don't set engine latch cvar to latched value.
else if ( cv && ( cv->flags & CVAR_LATCH ) && !( cv->flags & CVAR_VM_CREATED ) ) {
cv->flags |= flags;
cvar_modifiedFlags |= flags;
}
else {
cv = Cvar_Get(varName, defaultValue, flags | CVAR_VM_CREATED); cv = Cvar_Get(varName, defaultValue, flags | CVAR_VM_CREATED);
} }

View file

@ -3319,13 +3319,13 @@ 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_LATCH|CVAR_NORESTART ); fs_basegame = Cvar_Get ("fs_basegame", "", CVAR_INIT );
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_LATCH|CVAR_NORESTART|CVAR_SYSTEMINFO ); fs_gamedirvar = Cvar_Get ("fs_game", "", CVAR_INIT|CVAR_SYSTEMINFO );
if (!gameName[0]) { if (!gameName[0]) {
Cvar_ForceReset( "com_basegame" ); Cvar_ForceReset( "com_basegame" );
@ -3430,6 +3430,8 @@ 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
@ -4077,10 +4079,17 @@ 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 (com_basegame->latchedString || fs_basegame->latchedString || fs_gamedirvar->latchedString) if(fs_gamedirvar->modified)
{ {
Com_GameRestart(checksumFeed, disconnect); if(FS_FilenameCompare(lastValidGame, fs_gamedirvar->string) &&
return qtrue; (*lastValidGame || FS_FilenameCompare(fs_gamedirvar->string, com_basegame->string)) &&
(*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)