Re: Don't allow newlines in URLs to end up in FTP commands

Hrvoje Niksic <[email protected]> writes:

> A fix that applies to 1.9.1 follows in a separate mail.
> Distributors of Wget will probably want to make sure to include the
> appropriate patch.

Here is that fix.

2005-05-07  Hrvoje Niksic  <[email protected]>

	* ftp-basic.c (ftp_request): Prevent newlines in VALUE causing
	inadvertent sending of multiple FTP commands.

--- wget-1.9.1.pristine/src/ftp-basic.c	2003-11-08 20:17:55.000000000 +0100
+++ wget-1.9.1/src/ftp-basic.c	2005-05-07 03:08:02.306330903 +0200
@@ -116,9 +116,30 @@
 static char *
 ftp_request (const char *command, const char *value)
 {
-  char *res = (char *)xmalloc (strlen (command)
-                               + (value ? (1 + strlen (value)) : 0)
-                               + 2 + 1);
+  char *res;
+
+  /* Check for newlines in VALUE (possibly injected by the %0A URL
+     escape) making the callers inadvertently send multiple FTP
+     commands at once.  Without this check an attacker could
+     intentionally redirect to ftp://server/fakedir%0Acommand.../ and
+     execute arbitrary FTP command on a remote FTP server.  */
+  if (value && strpbrk (value, "\r\n"))
+    {
+      /* Copy VALUE to the stack and modify CR/LF to space. */
+      char *defanged, *p;
+      STRDUP_ALLOCA (defanged, value);
+      for (p = defanged; *p; p++)
+	if (*p == '\r' || *p == '\n')
+	  *p = ' ';
+      DEBUGP (("\nDetected newlines in %s \"%s\"; changing to %s \"%s\"\n",
+	       command, value, command, defanged));
+      /* Make VALUE point to the defanged copy of the string. */
+      value = defanged;
+    }
+
+  res = (char *)xmalloc (strlen (command)
+			 + (value ? (1 + strlen (value)) : 0)
+			 + 2 + 1);
   sprintf (res, "%s%s%s\r\n", command, value ? " " : "", value ? value : "");
   if (opt.server_response)
     {