From 373af010920537ba949c113163ddf0537cb30078 Mon Sep 17 00:00:00 2001 From: James R Date: Fri, 14 Oct 2022 18:20:52 -0700 Subject: [PATCH 1/3] Add startswith and endswith, functions that compare the beginning or ending of a string --- src/doomtype.h | 3 +++ src/string.c | 16 ++++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/src/doomtype.h b/src/doomtype.h index 5ddd9ae44..95871e98c 100644 --- a/src/doomtype.h +++ b/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 diff --git a/src/string.c b/src/string.c index 5534a3f0c..f48788e35 100644 --- a/src/string.c +++ b/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); +} From b1a86b0b340fce3d0b92493bca6db55878f2eafa Mon Sep 17 00:00:00 2001 From: James R Date: Fri, 14 Oct 2022 17:40:13 -0700 Subject: [PATCH 2/3] Disallow adding files with absolute path or traversing upward (Except as part of srb2home, srb2path or addons_folder -- this lets addons menu work, primarily.) - disallowed when using addfile or addfolder - security check for xcmd receive --- src/d_main.c | 71 +++++++++++++++++++++++++++++++++++++++++++++++++++ src/d_main.h | 2 ++ src/p_setup.c | 43 +++++++++++++++---------------- 3 files changed, 94 insertions(+), 22 deletions(-) diff --git a/src/d_main.c b/src/d_main.c index fa9e21337..6e76672e0 100644 --- a/src/d_main.c +++ b/src/d_main.c @@ -1689,3 +1689,74 @@ 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; +} diff --git a/src/d_main.h b/src/d_main.h index 8189a9f2b..7760351f3 100644 --- a/src/d_main.h +++ b/src/d_main.h @@ -44,6 +44,8 @@ void D_ProcessEvents(void); const char *D_Home(void); +boolean D_IsPathAllowed(const char *path); + // // BASE LEVEL // diff --git a/src/p_setup.c b/src/p_setup.c index 03a702b30..132dc4259 100644 --- a/src/p_setup.c +++ b/src/p_setup.c @@ -7860,8 +7860,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; @@ -7883,6 +7885,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: @@ -8051,34 +8059,25 @@ static boolean P_LoadAddon(UINT16 wadnum, UINT16 numlumps) return true; } -boolean P_AddWadFile(const char *wadfilename) +static boolean P_CheckAddonPath(const char *path) { - UINT16 numlumps, wadnum; - - // Init file. - if ((numlumps = W_InitFile(wadfilename, false, false)) == INT16_MAX) + if (!D_IsPathAllowed(path)) { - refreshdirmenu |= REFRESHDIR_NOTLOADED; + CONS_Alert(CONS_WARNING, "%s: tried to add file, location is not allowed\n", path); return false; } - else - wadnum = (UINT16)(numwadfiles-1); - return P_LoadAddon(wadnum, numlumps); + return true; +} + +boolean P_AddWadFile(const char *wadfilename) +{ + return P_CheckAddonPath(wadfilename) && + 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 P_CheckAddonPath(folderpath) && + P_LoadAddon(W_InitFolder(folderpath, false, false)); } From 76879299f98b2b91f9a7df661e17c137ff2f6598 Mon Sep 17 00:00:00 2001 From: James R Date: Fri, 14 Oct 2022 22:10:24 -0700 Subject: [PATCH 3/3] Restrict exec path to srb2 directories --- src/command.c | 4 ++++ src/d_main.c | 11 +++++++++++ src/d_main.h | 1 + src/m_misc.c | 1 + src/p_setup.c | 15 ++------------- 5 files changed, 19 insertions(+), 13 deletions(-) diff --git a/src/command.c b/src/command.c index dae4dc7b1..f8b587328 100644 --- a/src/command.c +++ b/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); diff --git a/src/d_main.c b/src/d_main.c index 6e76672e0..5b102d623 100644 --- a/src/d_main.c +++ b/src/d_main.c @@ -1760,3 +1760,14 @@ boolean D_IsPathAllowed(const char *path) 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; +} diff --git a/src/d_main.h b/src/d_main.h index 7760351f3..cc06f5f61 100644 --- a/src/d_main.h +++ b/src/d_main.h @@ -45,6 +45,7 @@ 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 diff --git a/src/m_misc.c b/src/m_misc.c index 6c346e5a1..fca0474eb 100644 --- a/src/m_misc.c +++ b/src/m_misc.c @@ -467,6 +467,7 @@ void Command_SaveConfig_f(void) CONS_Printf(M_GetText("saveconfig [-silent] : save config to a file\n")); return; } + strcpy(tmpstr, COM_Argv(1)); FIL_ForceExtension(tmpstr, ".cfg"); diff --git a/src/p_setup.c b/src/p_setup.c index 132dc4259..45813e04d 100644 --- a/src/p_setup.c +++ b/src/p_setup.c @@ -8059,25 +8059,14 @@ static boolean P_LoadAddon(UINT16 numlumps) return true; } -static boolean P_CheckAddonPath(const char *path) -{ - if (!D_IsPathAllowed(path)) - { - CONS_Alert(CONS_WARNING, "%s: tried to add file, location is not allowed\n", path); - return false; - } - - return true; -} - boolean P_AddWadFile(const char *wadfilename) { - return P_CheckAddonPath(wadfilename) && + return D_CheckPathAllowed(wadfilename, "tried to add file") && P_LoadAddon(W_InitFile(wadfilename, false, false)); } boolean P_AddFolder(const char *folderpath) { - return P_CheckAddonPath(folderpath) && + return D_CheckPathAllowed(folderpath, "tried to add folder") && P_LoadAddon(W_InitFolder(folderpath, false, false)); }