Quake2 filesytem bug

Discuss programming topics for the various GPL'd game engine sources.
Post Reply
Knightmare
Posts: 63
Joined: Thu Feb 09, 2012 1:55 am

Quake2 filesytem bug

Post 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.
Spike
Posts: 2914
Joined: Fri Nov 05, 2004 3:12 am
Location: UK
Contact:

Re: Quake2 filesytem bug

Post by Spike »

presumably this issue includes stuff like "." too, or 'set game "../../..";record autoexec.bat' or other horribly outdated example paths...
jitspoe
Posts: 217
Joined: Mon Jan 17, 2005 5:27 am

Re: Quake2 filesytem bug

Post 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
	}
Post Reply