Move file name check to prevent spurious "refusing to download messages

Moving the check under the "do we have the file localy" check prevents
spurious "Refusing to download a path with .." messages with some game
data. The tank commander skin is one example. This change has no
security impact since FS_LoadFile() just opens and closes the file.

While at it tighten the condition to prevent pathes with colons (this
condition is added at the server side, too) and pathes starting with
slashes and dots.
This commit is contained in:
Yamagi Burmeister 2015-10-27 17:38:28 +01:00
parent 17e791e528
commit 1d709e5e27
2 changed files with 7 additions and 9 deletions

View file

@ -445,14 +445,6 @@ CL_CheckOrDownloadFile(char *filename)
char name[MAX_OSPATH];
char *ptr;
// FIXME: we should probably also forbid paths starting with '/' or '\\' or "C:\"
// (or any other drive name) because in the end FS_LoadFile() will fallback to fopen()!
if (strstr(filename, ".."))
{
Com_Printf("Refusing to download a path with ..: %s\n", filename);
return true;
}
/* fix backslashes - this is mostly für UNIX comaptiblity */
while ((ptr = strchr(filename, '\\')))
{
@ -465,6 +457,12 @@ CL_CheckOrDownloadFile(char *filename)
return true;
}
if (strstr(filename, "..") || strstr(name, ":") || (*name == '.') || (*name == '/'))
{
Com_Printf("Refusing to download a path with ..: %s\n", filename);
return true;
}
strcpy(cls.downloadname, filename);
/* download to a temp name, and only rename

View file

@ -308,7 +308,7 @@ SV_BeginDownload_f(void)
/* hacked by zoid to allow more conrol over download
first off, no .. or global allow check */
if (strstr(name, "..") || strstr(name, "\\") || !allow_download->value
if (strstr(name, "..") || strstr(name, "\\") || strstr(name, ":") || !allow_download->value
/* leading dot is no good */
|| (*name == '.')
/* leading slash bad as well, must be in subdir */