Quake2 filesytem bug

[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]

presumably this issue includes stuff like "." too, or 'set game "../../..";record autoexec.bat' or other horribly outdated example paths...

[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
   }