Merge branch '22-safe-addfolder' into 'public_next'

Path security checks for addfile, addfolder and exec

See merge request STJr/SRB2Internal!633

src/command.c

@@ -34,6 +34,7 @@
 #include "lua_script.h"
 #include "d_netfil.h" // findfile
 #include "r_data.h" // Color_cons_t
+#include "d_main.h" // D_IsPathAllowed
 
 //========
 // protos.
@@ -770,6 +771,9 @@ static void COM_Exec_f(void)
 		return;
 	}
 
+	if (!D_CheckPathAllowed(COM_Argv(1), "tried to exec"))
+		return;
+
 	// load file
 	// Try with Argv passed verbatim first, for back compat
 	FIL_ReadFile(COM_Argv(1), &buf);

src/d_main.c

@@ -1777,3 +1777,85 @@ const char *D_Home(void)
 	if (usehome) return userhome;
 	else return NULL;
 }
+
+static boolean check_top_dir(const char **path, const char *top)
+{
+	// empty string does NOT match
+	if (!strcmp(top, ""))
+		return false;
+
+	if (!startswith(*path, top))
+		return false;
+
+	*path += strlen(top);
+
+	// if it doesn't already end with a path separator,
+	// check if a separator follows
+	if (!endswith(top, PATHSEP))
+	{
+		if (startswith(*path, PATHSEP))
+			*path += strlen(PATHSEP);
+		else
+			return false;
+	}
+
+	return true;
+}
+
+static int cmp_strlen_desc(const void *a, const void *b)
+{
+	return ((int)strlen(*(const char*const*)b) - (int)strlen(*(const char*const*)a));
+}
+
+boolean D_IsPathAllowed(const char *path)
+{
+	const char *paths[] = {
+		srb2home,
+		srb2path,
+		cv_addons_folder.string
+	};
+
+	const size_t n_paths = sizeof paths / sizeof *paths;
+
+	size_t i;
+
+	// Sort folder paths by longest to shortest so
+	// overlapping paths work. E.g.:
+	// Path 1: /home/james/.srb2/addons
+	// Path 2: /home/james/.srb2
+	qsort(paths, n_paths, sizeof *paths, cmp_strlen_desc);
+
+	// These paths are allowed to be absolute
+	// path is offset so ".." can be checked only in the
+	// rest of the path
+	for (i = 0; i < n_paths; ++i)
+	{
+		if (check_top_dir(&path, paths[i]))
+			break;
+	}
+
+	// Only if none of the presets matched
+	if (i == n_paths)
+	{
+		// Cannot be an absolute path
+		if (M_IsPathAbsolute(path))
+			return false;
+	}
+
+	// Cannot traverse upwards
+	if (strstr(path, ".."))
+		return false;
+
+	return true;
+}
+
+boolean D_CheckPathAllowed(const char *path, const char *why)
+{
+	if (!D_IsPathAllowed(path))
+	{
+		CONS_Alert(CONS_WARNING, "%s: %s, location is not allowed\n", why, path);
+		return false;
+	}
+
+	return true;
+}

src/d_main.h

@@ -44,6 +44,9 @@ void D_ProcessEvents(void);
 
 const char *D_Home(void);
 
+boolean D_IsPathAllowed(const char *path);
+boolean D_CheckPathAllowed(const char *path, const char *why);
+
 //
 // BASE LEVEL
 //

src/doomtype.h

@@ -107,6 +107,9 @@ typedef long ssize_t;
 char *strcasestr(const char *in, const char *what);
 #define stristr strcasestr
 
+int startswith (const char *base, const char *tag);
+int endswith (const char *base, const char *tag);
+
 #if defined (macintosh) //|| defined (__APPLE__) //skip all boolean/Boolean crap
 	#define true 1
 	#define false 0

src/m_misc.c

@@ -468,6 +468,7 @@ void Command_SaveConfig_f(void)
 		CONS_Printf(M_GetText("saveconfig <filename[.cfg]> [-silent] : save config to a file\n"));
 		return;
 	}
+
 	strcpy(tmpstr, COM_Argv(1));
 	FIL_ForceExtension(tmpstr, ".cfg");
 

src/p_setup.c

@@ -7896,8 +7896,10 @@ static lumpinfo_t* FindFolder(const char *folName, UINT16 *start, UINT16 *end, l
 // Add a wadfile to the active wad files,
 // replace sounds, musics, patches, textures, sprites and maps
 //
-static boolean P_LoadAddon(UINT16 wadnum, UINT16 numlumps)
+static boolean P_LoadAddon(UINT16 numlumps)
 {
+	const UINT16 wadnum = (UINT16)(numwadfiles-1);
+
 	size_t i, j, sreplaces = 0, mreplaces = 0, digmreplaces = 0;
 	char *name;
 	lumpinfo_t *lumpinfo;
@@ -7919,6 +7921,12 @@ static boolean P_LoadAddon(UINT16 wadnum, UINT16 numlumps)
 //	UINT16 flaPos, flaNum = 0;
 //	UINT16 mapPos, mapNum = 0;
 
+	if (numlumps == INT16_MAX)
+	{
+		refreshdirmenu |= REFRESHDIR_NOTLOADED;
+		return false;
+	}
+
 	switch(wadfiles[wadnum]->type)
 	{
 	case RET_PK3:
@@ -8089,32 +8097,12 @@ static boolean P_LoadAddon(UINT16 wadnum, UINT16 numlumps)
 
 boolean P_AddWadFile(const char *wadfilename)
 {
-	UINT16 numlumps, wadnum;
-
-	// Init file.
-	if ((numlumps = W_InitFile(wadfilename, false, false)) == INT16_MAX)
-	{
-		refreshdirmenu |= REFRESHDIR_NOTLOADED;
-		return false;
-	}
-	else
-		wadnum = (UINT16)(numwadfiles-1);
-
-	return P_LoadAddon(wadnum, numlumps);
+	return D_CheckPathAllowed(wadfilename, "tried to add file") &&
+		P_LoadAddon(W_InitFile(wadfilename, false, false));
 }
 
 boolean P_AddFolder(const char *folderpath)
 {
-	UINT16 numlumps, wadnum;
-
-	// Init file.
-	if ((numlumps = W_InitFolder(folderpath, false, false)) == INT16_MAX)
-	{
-		refreshdirmenu |= REFRESHDIR_NOTLOADED;
-		return false;
-	}
-	else
-		wadnum = (UINT16)(numwadfiles-1);
-
-	return P_LoadAddon(wadnum, numlumps);
+	return D_CheckPathAllowed(folderpath, "tried to add folder") &&
+		P_LoadAddon(W_InitFolder(folderpath, false, false));
 }

src/string.c

@@ -52,3 +52,19 @@ size_t strlcpy(char *dst, const char *src, size_t siz)
 #endif
 
 #include "strcasestr.c"
+
+int startswith(const char *path, const char *tag)
+{
+	return !strncmp(path, tag, strlen(tag));
+}
+
+int endswith(const char *base, const char *tag)
+{
+	const size_t base_length = strlen(base);
+	const size_t tag_length = strlen(tag);
+
+	if (tag_length > base_length)
+		return false;
+
+	return !memcmp(&base[base_length - tag_length], tag, tag_length);
+}