Page 1 of 1

Quake2 filesytem bug

Posted: Wed Mar 04, 2015 7:41 pm
by Knightmare
While testing out our new HTTP downloading code in Daikatana v1.3, I found a filesytem bug that affects all Q2 engines.

If you run the game with a mod dir set (+set game <moddir>) and then are forced (usually by connecting to a multiplayer server) back to a non-mod dir (game is now set to ""), then the save path used for .cfg files, savegames, and downloads is set to the game root, instead of the base/main data dir.

The problem is in qcommon/files.c->FS_SetGamedir():

Code: Select all

	Com_sprintf (fs_gamedir, sizeof(fs_gamedir), "%s/%s", fs_basedir->string, dir);
This doesn't check if dir is a zero length string. If it is, then the write path gets set to the game root.

Here's my fix that takes this into account and sets it back to the hardcoded basedir:

Code: Select all

	if (*dir == 0)	// Knightmare- set to basedir if a blank dir is passed
		Com_sprintf (fs_gamedir, sizeof(fs_gamedir), "%s/"BASEDIRNAME, fs_basedir->string);
	else
		Com_sprintf (fs_gamedir, sizeof(fs_gamedir), "%s/%s", fs_basedir->string, dir);
I'm posting this here so anyone else working on a Q2 engine project knows about this.

Re: Quake2 filesytem bug

Posted: Wed Mar 04, 2015 7:48 pm
by Spike
presumably this issue includes stuff like "." too, or 'set game "../../..";record autoexec.bat' or other horribly outdated example paths...

Re: Quake2 filesytem bug

Posted: Thu Mar 05, 2015 12:38 am
by jitspoe
There are already checks for that (assuming this is in vanilla q2):

Code: Select all

	if (strstr(dir, "..") || strstr(dir, "/")
		|| strstr(dir, "\\") || strstr(dir, ":"))
	{
		Com_Printf("Gamedir should be a single filename, not a path.\n");
		return;
	}
I guess I must have had to deal with something similar. I found these in my source:

Code: Select all

char *FS_Gamedir (void)
{
	if (*fs_gamedir && !Q_streq(fs_gamedir, "./")) // jit
		return fs_gamedir;
	else
		return BASEDIRNAME;
}

Code: Select all

	if (Q_streq(dir, BASEDIRNAME) || (*dir == 0))
	{
		Cvar_FullSet("gamedir", BASEDIRNAME, CVAR_SERVERINFO | CVAR_NOSET, true); // jit, always display gamedir as "pball"
		Cvar_FullSet("game", BASEDIRNAME, CVAR_LATCH | CVAR_SERVERINFO, true); // jit
	}