- overhaul of savegame path management.

* use a different subfolder for each IWAD's saves.
* do not allow load and save CCMDs to escape the save folder. Absolute paths and '..' are being blocked now.
* unified savegame path and filename generation in one single function. All ad-hoc file name generation was replaced.
* -loadgame will also use the designated savegame folder now.

src/common/menu/savegamemanager.cpp

@@ -46,8 +46,11 @@
 #include "findfile.h"
 #include "v_draw.h"
 #include "savegamemanager.h"
+#include "m_argv.h"
+#include "i_specialpaths.h"
 
-
+CVAR(String, save_dir, "", CVAR_ARCHIVE | CVAR_GLOBALCONFIG);
+FString SavegameFolder;
 
 //=============================================================================
 //
@@ -538,3 +541,56 @@ DEFINE_FIELD_X(SavegameManager, FSavegameManagerBase, WindowSize);
 DEFINE_FIELD_X(SavegameManager, FSavegameManagerBase, quickSaveSlot);
 DEFINE_FIELD_X(SavegameManager, FSavegameManagerBase, SaveCommentString);
 
+//=============================================================================
+//
+// todo: cache this - it never changes once set up.
+//
+//=============================================================================
+
+FString G_GetSavegamesFolder()
+{
+	FString name;
+	bool usefilter;
+
+	if (const char* const dir = Args->CheckValue("-savedir"))
+	{
+		name = dir;
+		usefilter = false; //-savedir specifies an absolute save directory path.
+	}
+	else
+	{
+		name = **save_dir ? save_dir : M_GetSavegamesPath();
+		usefilter = true;
+	}
+
+	const size_t len = name.Len();
+	if (len > 0)
+	{
+		FixPathSeperator(name);
+		if (name[len - 1] != '/')
+			name << '/';
+	}
+
+	if (usefilter && SavegameFolder.IsNotEmpty())
+		name << SavegameFolder << '/';
+
+	name = NicePath(name);
+	CreatePath(name);
+	return name;
+}
+
+//=============================================================================
+//
+//
+//
+//=============================================================================
+
+FString G_BuildSaveName(const char* prefix)
+{
+	FString name = G_GetSavegamesFolder() + prefix;
+	DefaultExtension(name, "." SAVEGAME_EXT); // only add an extension if the prefix doesn't have one already.
+	name = NicePath(name);
+	name.Substitute("\\", "/");
+	return name;
+}
+

src/common/menu/savegamemanager.h

@@ -59,3 +59,6 @@ public:
 
 };
 
+extern FString SavegameFolder;	// specifies a subdirectory for the current IWAD.
+FString G_GetSavegamesFolder();
+FString G_BuildSaveName(const char* prefix);

src/console/c_cmds.cpp

@@ -70,6 +70,7 @@
 #include "texturemanager.h"
 #include "v_draw.h"
 #include "d_main.h"
+#include "savegamemanager.h"
 
 extern FILE *Logfile;
 extern bool insave;
@@ -639,6 +640,11 @@ UNSAFE_CCMD (load)
 		Printf("saving to an absolute path is not allowed\n");
 		return;
 	}
+	if (fname.IndexOf("..") > 0)
+	{
+		Printf("'..' not allowed in file names\n");
+		return;
+	}
 #ifdef _WIN32
 	// block all invalid characters for Windows file names
 	if (fname.IndexOfAny(":?*<>|") >= 0)
@@ -647,8 +653,7 @@ UNSAFE_CCMD (load)
 		return;
 	}
 #endif
-	fname = G_BuildSaveName(fname, -1);
+	fname = G_BuildSaveName(fname);
-	DefaultExtension(fname, "." SAVEGAME_EXT);
 	G_LoadGame (fname);
 }
 
@@ -674,6 +679,11 @@ UNSAFE_CCMD(save)
 		Printf("saving to an absolute path is not allowed\n");
 		return;
 	}
+	if (fname.IndexOf("..") > 0)
+	{
+		Printf("'..' not allowed in file names\n");
+		return;
+	}
 #ifdef _WIN32
 	// block all invalid characters for Windows file names
 	if (fname.IndexOfAny(":?*<>|") >= 0)
@@ -682,8 +692,7 @@ UNSAFE_CCMD(save)
 		return;
 	}
 #endif
-    fname = G_BuildSaveName(fname, -1);
+    fname = G_BuildSaveName(fname);
-	DefaultExtension(fname, "." SAVEGAME_EXT);
 	G_SaveGame (fname, argv.argc() > 2 ? argv[2] : argv[1]);
 }
 

src/d_main.cpp

@@ -3021,6 +3021,7 @@ static FILE* D_GetHashFile()
 
 static int D_InitGame(const FIWADInfo* iwad_info, TArray<FString>& allwads, TArray<FString>& pwads)
 {
+	SavegameFolder = iwad_info->Autoname;
 	gameinfo.gametype = iwad_info->gametype;
 	gameinfo.flags = iwad_info->flags;
 	gameinfo.nokeyboardcheats = iwad_info->nokeyboardcheats;
@@ -3428,10 +3429,8 @@ static int D_InitGame(const FIWADInfo* iwad_info, TArray<FString>& allwads, TArr
 		v = Args->CheckValue ("-loadgame");
 		if (v)
 		{
-			FString file(v);
+			FString file = G_BuildSaveName(v);
-			FixPathSeperator (file);
+			G_LoadGame(file);
-			DefaultExtension (file, "." SAVEGAME_EXT);
-			G_LoadGame (file);
 		}
 
 		v = Args->CheckValue("-playdemo");
@@ -3612,9 +3611,7 @@ static int D_DoomMain_Internal (void)
 	v = Args->CheckValue("-loadgame");
 	if (v)
 	{
-		FString file(v);
+		FString file = G_BuildSaveName(v);
-		FixPathSeperator(file);
-		DefaultExtension(file, "." SAVEGAME_EXT);
 		if (!FileExists(file))
 		{
 			I_FatalError("Cannot find savegame %s", file.GetChars());

src/d_net.cpp

@@ -71,6 +71,7 @@
 #include "screenjob.h"
 #include "d_main.h"
 #include "i_interface.h"
+#include "savegamemanager.h"
 
 EXTERN_CVAR (Int, disableautosave)
 EXTERN_CVAR (Int, autosavecount)
@@ -2457,21 +2458,8 @@ void Net_DoCommand (int type, uint8_t **stream, int player)
 			{
 				// Paths sent over the network will be valid for the system that sent
 				// the save command. For other systems, the path needs to be changed.
-				const char *fileonly = savegamefile.GetChars();
+				FString basename = ExtractFileBase(savegamefile, true);
-				const char *slash = strrchr (fileonly, '\\');
+				savegamefile = G_BuildSaveName (basename);
-				if (slash != NULL)
-				{
-					fileonly = slash + 1;
-				}
-				slash = strrchr (fileonly, '/');
-				if (slash != NULL)
-				{
-					fileonly = slash + 1;
-				}
-				if (fileonly != savegamefile.GetChars())
-				{
-					savegamefile = G_BuildSaveName (fileonly, -1);
-				}
 			}
 		}
 		gameaction = ga_savegame;

src/g_game.cpp

@@ -119,7 +119,6 @@ CVAR (Int, deathmatch, 0, CVAR_SERVERINFO|CVAR_LATCH);
 CVAR (Bool, chasedemo, false, 0);
 CVAR (Bool, storesavepic, true, CVAR_ARCHIVE|CVAR_GLOBALCONFIG)
 CVAR (Bool, longsavemessages, false, CVAR_ARCHIVE|CVAR_GLOBALCONFIG)
-CVAR (String, save_dir, "", CVAR_ARCHIVE|CVAR_GLOBALCONFIG);
 CVAR (Bool, cl_waitforsave, true, CVAR_ARCHIVE | CVAR_GLOBALCONFIG);
 CVAR (Bool, enablescriptscreenshot, false, CVAR_ARCHIVE | CVAR_GLOBALCONFIG);
 EXTERN_CVAR (Float, con_midtime);
@@ -2170,61 +2169,10 @@ void G_SaveGame (const char *filename, const char *description)
 	}
 }
 
-FString G_BuildSaveName (const char *prefix, int slot)
-{
-	FString name;
-	FString leader;
-	const char *slash = "";
-
-	leader = Args->CheckValue ("-savedir");
-	if (leader.IsEmpty())
-	{
-		leader = save_dir;
-		if (leader.IsEmpty())
-		{
-			leader = M_GetSavegamesPath();
-		}
-	}
-	size_t len = leader.Len();
-	if (leader[0] != '\0' && leader[len-1] != '\\' && leader[len-1] != '/')
-	{
-		slash = "/";
-	}
-	name << leader << slash;
-	name = NicePath(name);
-	CreatePath(name);
-	name << prefix;
-	if (slot >= 0)
-	{
-		name.AppendFormat("%d." SAVEGAME_EXT, slot);
-	}
-	return name;
-}
-
 CCMD(opensaves)
 {
-	FString name;
+	FString name = G_GetSavegamesFolder();
-	FString leader;
-	const char *slash = "";
-
-	leader = Args->CheckValue ("-savedir");
-	if (leader.IsEmpty())
-	{
-		leader = save_dir;
-		if (leader.IsEmpty())
-		{
-			leader = M_GetSavegamesPath();
-		}
-	}
-	size_t len = leader.Len();
-	if (leader[0] != '\0' && leader[len-1] != '\\' && leader[len-1] != '/')
-	{
-		slash = "/";
-	}
-	name << leader << slash;
-	name = NicePath(name);
 	CreatePath(name);
-
 	I_OpenShellFolder(name);
 }
 
@@ -2263,7 +2211,7 @@ void G_DoAutoSave ()
 	num.Int = nextautosave;
 	autosavenum->ForceSet (num, CVAR_Int);
 
-	file = G_BuildSaveName ("auto", nextautosave);
+	file = G_BuildSaveName(FStringf("auto%02d", nextautosave));
 
 	// The hint flag is only relevant on the primary level.
 	if (!(primaryLevel->flags2 & LEVEL2_NOAUTOSAVEHINT))
@@ -2302,7 +2250,7 @@ void G_DoQuickSave ()
 	num.Int = lastquicksave;
 	quicksavenum->ForceSet (num, CVAR_Int);
 
-	file = G_BuildSaveName ("quick", lastquicksave);
+	file = G_BuildSaveName(FStringf("quick%02d", nextautosave));
 
 	readableTime = myasctime ();
 	description.Format("Quicksave %s", readableTime);
@@ -2377,7 +2325,7 @@ void G_DoSaveGame (bool okForQuicksave, bool forceQuicksave, FString filename, c
 
 	if (demoplayback)
 	{
-		filename = G_BuildSaveName ("demosave." SAVEGAME_EXT, -1);
+		filename = G_BuildSaveName ("demosave");
 	}
 
 	if (cl_waitforsave)

src/g_game.h

@@ -81,8 +81,6 @@ bool G_Responder (event_t*	ev);
 void G_ScreenShot (const char* filename);
 void G_StartSlideshow(FLevelLocals *Level, FName whichone);
 
-FString G_BuildSaveName (const char *prefix, int slot);
-
 class FSerializer;
 bool G_CheckSaveGameWads (FSerializer &arc, bool printwarn);
 

src/menu/loadsavemenu.cpp

@@ -68,14 +68,14 @@ void FSavegameManager::ReadSaveStrings()
 
 		LastSaved = LastAccessed = -1;
 		quickSaveSlot = nullptr;
-		filter = G_BuildSaveName("*." SAVEGAME_EXT, -1);
+		filter = G_BuildSaveName("*");
 		filefirst = I_FindFirst(filter.GetChars(), &c_file);
 		if (filefirst != ((void *)(-1)))
 		{
 			do
 			{
 				// I_FindName only returns the file's name and not its full path
-				FString filepath = G_BuildSaveName(I_FindName(&c_file), -1);
+				FString filepath = G_BuildSaveName(I_FindName(&c_file));
 
 				std::unique_ptr<FResourceFile> savegame(FResourceFile::OpenResourceFile(filepath, true, true));
 				if (savegame != nullptr)
@@ -252,7 +252,7 @@ FString FSavegameManager::ExtractSaveComment(FSerializer &arc)
 
 FString FSavegameManager::BuildSaveName(const char* prefix, int slot)
 {
-	return G_BuildSaveName(prefix, slot);
+	return G_BuildSaveName(FStringf("%s%02d", prefix, slot));
 }
 
 //=============================================================================