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

Hrvoje Niksic <[email protected]> Sat, 07 May 2005 03:04:54 +0200
Newsgroups gmane.comp.web.wget.patches,gmane.comp.web.wget.general
Message-ID <[email protected]>
A newline in an FTP URL can causes Wget to effectively send the
URL-specified command to the server.  Since URL may come from the
network, this can be construed as a vulnerability.

A separate 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.


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

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

Index: src/ftp-basic.c
===================================================================
RCS file: /pack/anoncvs/wget/src/ftp-basic.c,v
retrieving revision 1.44
diff -u -r1.44 ftp-basic.c
--- src/ftp-basic.c	2005/05/05 10:10:51	1.44
+++ src/ftp-basic.c	2005/05/07 01:04:11
@@ -103,7 +103,27 @@
 {
   char *res;
   if (value)
-    res = concat_strings (command, " ", value, "\r\n", (char *) 0);
+    {
+      /* 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 (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, escnonprint (value), command, escnonprint (defanged)));
+	  /* Make VALUE point to the defanged copy of the string. */
+	  value = defanged;
+	}
+      res = concat_strings (command, " ", value, "\r\n", (char *) 0);
+    }
   else
     res = concat_strings (command, "\r\n", (char *) 0);
   if (opt.server_response)