From: havoc Date: Fri, 17 Dec 2004 15:51:24 +0000 (+0000) Subject: removed dangerous filename checks from FRIK_FILE code X-Git-Tag: xonotic-v0.1.0preview~5291 X-Git-Url: https://de.git.xonotic.org/?a=commitdiff_plain;h=2ea63a984d8814f1a639e854ff1f663549d21c5a;p=xonotic%2Fdarkplaces.git removed dangerous filename checks from FRIK_FILE code added FS_CheckNastyPath function which is called by FS_Open, so now dangerous paths are rejected everywhere git-svn-id: svn://svn.icculus.org/twilight/trunk/darkplaces@4848 d7cf8633-e32d-0410-b094-e92efae38249 --- diff --git a/fs.c b/fs.c index 157a9c7b..82774b7a 100644 --- a/fs.c +++ b/fs.c @@ -1126,6 +1126,39 @@ qfile_t *FS_OpenRead (const char *path, int offs, int len) return file; } +/* +==================== +FS_CheckNastyPath + +Return true if the path should be rejected due to one of the following: +1: path elements that are non-portable +2: path elements that would allow access to files outside the game directory, + or are just not a good idea for a mod to be using. +==================== +*/ +int FS_CheckNastyPath (const char *path) +{ + // Windows: don't allow \ in filenames (windows-only), period. + // (on Windows \ is a directory separator, but / is also supported) + if (strstr(path, "\\")) + return 1; // non-portable + // Mac: don't allow Mac-only filenames - : is a directory separator + // instead of /, but we rely on / working already, so there's no reason to + // support a Mac-only path + // Amiga and Windows: : tries to go to root of drive + if (strstr(path, ":")) + return 1; // non-portable attempt to go to root of drive + // Amiga: // is parent directory + if (strstr(path, "//")) + return 1; // non-portable attempt to go to parent directory + // all: don't allow going to current directory (./) or parent directory (../ or /../) + if (strstr(path, "./")) + return 2; // attempt to go to parent directory + // after all these checks we're pretty sure it's a / separated filename + // and won't do much if any harm + return false; +} + /* ==================== @@ -1317,6 +1350,12 @@ Open a file. The syntax is the same as fopen */ qfile_t* FS_Open (const char* filepath, const char* mode, qboolean quiet) { + if (FS_CheckNastyPath(filepath)) + { + Con_Printf("FS_Open(\"%s\", \"%s\", %s): nasty filename rejected\n", filepath, mode, quiet ? "true" : "false"); + return NULL; + } + // If the file is opened in "write" or "append" mode if (strchr (mode, 'w') || strchr (mode, 'a')) { diff --git a/pr_cmds.c b/pr_cmds.c index 6de3550b..a8886449 100644 --- a/pr_cmds.c +++ b/pr_cmds.c @@ -2789,7 +2789,7 @@ void PF_stof(void) //float(string filename, float mode) fopen = #110; // opens a file inside quake/gamedir/data/ (mode is FILE_READ, FILE_APPEND, or FILE_WRITE), returns fhandle >= 0 if successful, or fhandle < 0 if unable to open file for any reason void PF_fopen(void) { - int filenum, mode, i; + int filenum, mode; char *modestring, *filename; for (filenum = 0;filenum < MAX_PRFILES;filenum++) if (pr_files[filenum] == NULL) @@ -2818,21 +2818,7 @@ void PF_fopen(void) return; } filename = G_STRING(OFS_PARM0); - // control characters do not cause issues with any platforms I know of, but they are usually annoying to deal with - // ../ is parent directory on many platforms - // // is parent directory on Amiga - // / at the beginning of a path is root on unix, and parent directory on Amiga - // : is root of drive on Amiga (also used as a directory separator on Mac, but / works there too, so that's a bad idea) - // \ is a windows-ism (so it's naughty to use it, / works on all platforms) - for (i = 0;filename[i];i++) - { - if (filename[i] < ' ' || (filename[i] == '/' && filename[i+1] == '/') || (filename[i] == '.' && filename[i+1] == '.') || filename[i] == ':' || filename[i] == '\\' || filename[0] == '/') - { - Con_Printf("PF_fopen: dangerous/confusing/annoying/non-portable filename \"%s\" not allowed. (contains control characters or // or .. or : or \\ or begins with /)\n", filename); - G_FLOAT(OFS_RETURN) = -4; - return; - } - } + // -4 failure (dangerous/non-portable filename) removed, FS_Open checks pr_files[filenum] = FS_Open(va("data/%s", filename), modestring, false); if (pr_files[filenum] == NULL && modestring == "rb")