Refactor the download filter.

The first try didn't take into account that an evil server could
override the filter list by sending a stuff command. Fix this by
hardcoding the filters for .dll, .dylib and .so. Make sure that the
filters are always applied, either when the download is requested
through the `download` command or because game data is missing.

This is just a poor mans fix, trying to rule out an obvious way to
inject code into the client.

doc/040_cvarlist.md

@@ -146,9 +146,14 @@ it's `+set busywait 0` (setting the `busywait` cvar) and `-portable`
   preview.  `-1` - don't show animation. Defaults to `94` for show
   salute animation.
 
-* **cl_nodownload_list**: Whitespace seperated list of strings, files
-  having one these strings in their name are never downloaded. Set to
-  `.dll .dylib .so` by default.
+* **cl_nodownload_list**: Whitespace separated list of substrings, files
+  having one these strings in their name are never downloaded. Empty by
+  default. Note that some substrings are always forbidden, for security
+  reasons these cannot be overridden: '.dll', '.dylib' and '.so' to
+  prevent downloading of libraries which could be injected into the
+  Yamagi Quake II process. '..' or ':' inside filenames and '/' or '.'
+  at the beginning of filenames to prevent downloading files into
+  arbitrary directories.
 
 * **cl_r1q2_lightstyle**: Since the first release Yamagi Quake II used
   the R1Q2 colors for the dynamic lights of rockets. Set to `0` to get

src/client/cl_download.c

@@ -542,6 +542,49 @@ CL_DownloadFileName(char *dest, int destlen, char *fn)
 	}
 }
 
+/*
+ * Returns true if a file is filtered and
+ * should not be downloaded, false otherwise.
+ */
+static qboolean
+CL_DownloadFilter(const char *filename)
+{
+	if (FS_LoadFile(      (char *) filename, NULL) != -1)
+	{
+		/* it exists, no need to download */
+		return true;
+	}
+
+	if (strstr(filename, "..") || strstr(filename, ":") || (*filename == '.') || (*filename == '/'))
+	{
+		Com_Printf("Refusing to download a path containing '..' or ':' or starting with '.' or '/': %s\n", filename);
+		return true;
+	}
+
+	if (strstr(filename, ".dll") || strstr(filename, ".dylib") || strstr(filename, ".so"))
+	{
+		Com_Printf("Refusing to download a path containing '.dll', '.dylib' or '.so': %s\n", filename);
+		return true;
+	}
+
+	char *nodownload = strdup(cl_nodownload_list->string);
+	char *nodownload_token = strtok(nodownload, " ");
+	while (nodownload_token != NULL)
+	{
+		Com_Printf("Token: %s\n", nodownload_token);
+		if (Q_strcasestr(filename, nodownload_token))
+		{
+			Com_Printf("Filename is filtered by cl_nodownload_list: %s\n", filename);
+			free(nodownload);
+			return true;
+		}
+		nodownload_token = strtok(NULL, " ");
+	}
+	free(nodownload);
+
+	return false;
+}
+
 /*
  * Returns true if the file exists, otherwise it attempts
  * to start a download from the server.
@@ -559,29 +602,11 @@ CL_CheckOrDownloadFile(char *filename)
 		*ptr = '/';
 	}
 
-	if (FS_LoadFile(filename, NULL) != -1)
+	if (CL_DownloadFilter(filename))
 	{
-		/* it exists, no need to download */
 		return true;
 	}
 
-	if (strstr(filename, "..") || strstr(filename, ":") || (*filename == '.') || (*filename == '/'))
-	{
-		Com_Printf("Refusing to download a path with ..: %s\n", filename);
-		return true;
-	}
-
-	char *nodownload = strtok(cl_nodownload_list->string, " ");
-	while (nodownload != NULL)
-	{
-		if (Q_strcasestr(filename, nodownload))
-		{
-			Com_Printf("Filename is filtered by cl_nodownload_list: %s\n", filename);
-			return true;
-		}
-		nodownload = strtok(NULL, " ");
-	}
-
 #ifdef USE_CURL
 	if (!forceudp)
 	{
@@ -685,16 +710,8 @@ CL_Download_f(void)
 
 	Com_sprintf(filename, sizeof(filename), "%s", Cmd_Argv(1));
 
-	if (strstr(filename, ".."))
+	if (CL_DownloadFilter(filename))
 	{
-		Com_Printf("Refusing to download a path with ..\n");
-		return;
-	}
-
-	if (FS_LoadFile(filename, NULL) != -1)
-	{
-		/* it exists, no need to download */
-		Com_Printf("File already exists.\n");
 		return;
 	}
 

src/client/cl_main.c

@@ -518,7 +518,7 @@ CL_InitLocal(void)
 	cl_showfps = Cvar_Get("cl_showfps", "0", CVAR_ARCHIVE);
 	cl_showspeed = Cvar_Get("cl_showspeed", "0", CVAR_ARCHIVE);	
 	cl_laseralpha = Cvar_Get("cl_laseralpha", "0.3", 0);
-	cl_nodownload_list = Cvar_Get("cl_nodownload_list", ".dll .dylib .so", 0);
+	cl_nodownload_list = Cvar_Get("cl_nodownload_list", "", CVAR_ARCHIVE);
 
 	cl_upspeed = Cvar_Get("cl_upspeed", "200", 0);
 	cl_forwardspeed = Cvar_Get("cl_forwardspeed", "200", 0);