[svn] Fix escape chars in server response vulnerability. Server response is

now quoted to escape non-ASCII characters.

diff --git a/src/ChangeLog b/src/ChangeLog
index 277a0edb8b85f0d5e8294ae9ab996b45acb7702b..762067f029d576c6d0d78629b4807b0d5a885480 100644 (file)
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,35 @@
+2005-03-03  Hrvoje Niksic  <hniksic@xemacs.org>
+
+       * retr.c (retrieve_url): Escape location header.
+
+       * http.c (print_server_response_1): Escape server response when
+       printing it.
+       (gethttp): Escape host name, status message, location header, and
+       content type.
+       (http_loop): Escape error message from server.
+
+       * host.c (lookup_host): Escape host name when printing it.
+
+       * ftp.c (getftp): Escape user name when printing it.
+       (getftp): Escape remote file and directory for printing.
+       (getftp): Escape server listing when printing it.
+       (ftp_retrieve_list): Escape link name and file name.
+       (ftp_retrieve_glob): Escape file name.
+
+       * ftp-basic.c (ftp_response): Escape server response when printing
+       it.
+
+       * cookies.c (parse_set_cookies): Escape the cookie field when
+       printing it.
+       (parse_set_cookies): Escape contents of remote header.
+       (cookie_handle_set_cookie): Escape host name and cookie domain.
+
+       * connect.c (connect_to_ip): Escape the host name.
+
+       * log.c (escnonprint): New function, used for printing strings
+       coming from the server that possibly contain non-ASCII characters.
+       (escnonprint_uri): Ditto.
+
 2005-02-24  Hrvoje Niksic  <hniksic@xemacs.org>
 
        * ftp.c (getftp): Ditto.

diff --git a/src/connect.c b/src/connect.c
index ff7a741c2c198b6c2e348251595447bb178af474..40efc3630517bbaa55642ad6f9587898a22c1108 100644 (file)
--- a/src/connect.c
+++ b/src/connect.c
@@ -269,8 +269,8 @@ connect_to_ip (const ip_address *ip, int port, const char *print)
     {
       const char *txt_addr = pretty_print_address (ip);
       if (print && 0 != strcmp (print, txt_addr))
-       logprintf (LOG_VERBOSE,
-                  _("Connecting to %s|%s|:%d... "), print, txt_addr, port);
+       logprintf (LOG_VERBOSE, _("Connecting to %s|%s|:%d... "),
+                  escnonprint (print), txt_addr, port);
       else
        logprintf (LOG_VERBOSE, _("Connecting to %s:%d... "), txt_addr, port);
     }

diff --git a/src/cookies.c b/src/cookies.c
index 98ab423c1f7782598988341349c27bdcbe8f1c37..fe9761fde43fa30ff2a1d1f8032e312d7d7dbdc4 100644 (file)
--- a/src/cookies.c
+++ b/src/cookies.c
@@ -616,7 +616,8 @@ parse_set_cookies (const char *sc,
                    char *name;
                    BOUNDED_TO_ALLOCA (name_b, name_e, name);
                    logprintf (LOG_NOTQUIET,
-                              _("Error in Set-Cookie, field `%s'"), name);
+                              _("Error in Set-Cookie, field `%s'"),
+                              escnonprint (name));
                  }
                state = S_ERROR;
                break;
@@ -640,7 +641,7 @@ parse_set_cookies (const char *sc,
   if (!silent)
     logprintf (LOG_NOTQUIET,
               _("Syntax error in Set-Cookie: %s at position %d.\n"),
-              sc, p - sc);
+              escnonprint (sc), p - sc);
   return NULL;
 }
 \f
@@ -862,7 +863,7 @@ cookie_handle_set_cookie (struct cookie_jar *jar,
        {
          logprintf (LOG_NOTQUIET,
                     "Cookie coming from %s attempted to set domain to %s\n",
-                    host, cookie->domain);
+                    escnonprint (host), escnonprint (cookie->domain));
          xfree (cookie->domain);
          goto copy_domain;
        }

diff --git a/src/ftp-basic.c b/src/ftp-basic.c
index a246e37c8f5f5f5a16694f212962bcaa9801d6d6..011268b2d58b11ef2f372b7a0f201cc5b0483312 100644 (file)
--- a/src/ftp-basic.c
+++ b/src/ftp-basic.c
@@ -67,9 +67,9 @@ ftp_response (int fd, char **ret_line)
       if (!line)
        return FTPRERR;
       if (opt.server_response)
-        logputs (LOG_NOTQUIET, line);
+        logputs (LOG_NOTQUIET, escnonprint (line));
       else
-        DEBUGP (("%s", line));
+        DEBUGP (("%s", escnonprint (line)));
       if (ISDIGIT (line[0]) && ISDIGIT (line[1]) && ISDIGIT (line[2])
          && line[3] == ' ')
        {

diff --git a/src/ftp.c b/src/ftp.c
index c1e2d002c7cd7244e9032aab07385b6f29a4cb4f..47face7192b9a6aa11b63ee68a8d3670b2b745b5 100644 (file)
--- a/src/ftp.c
+++ b/src/ftp.c
@@ -292,7 +292,7 @@ getftp (struct url *u, wgint *len, wgint restval, ccon *con)
        con->csock = -1;
 
       /* Second: Login with proper USER/PASS sequence.  */
-      logprintf (LOG_VERBOSE, _("Logging in as %s ... "), user);
+      logprintf (LOG_VERBOSE, _("Logging in as %s ... "), escnonprint (user));
       if (opt.server_response)
        logputs (LOG_ALWAYS, "\n");
       err = ftp_login (csock, logname, passwd);
@@ -551,7 +551,7 @@ Error in server response, closing control connection.\n"));
            }
 
          if (!opt.server_response)
-           logprintf (LOG_VERBOSE, "==> CWD %s ... ", target);
+           logprintf (LOG_VERBOSE, "==> CWD %s ... ", escnonprint (target));
          err = ftp_cwd (csock, target);
          /* FTPRERR, WRITEFAILED, FTPNSFOD */
          switch (err)
@@ -575,7 +575,7 @@ Error in server response, closing control connection.\n"));
            case FTPNSFOD:
              logputs (LOG_VERBOSE, "\n");
              logprintf (LOG_NOTQUIET, _("No such directory `%s'.\n\n"),
-                        u->dir);
+                        escnonprint (u->dir));
              fd_close (csock);
              con->csock = -1;
              return err;
@@ -599,7 +599,7 @@ Error in server response, closing control connection.\n"));
       if (opt.verbose)
        {
           if (!opt.server_response)
-           logprintf (LOG_VERBOSE, "==> SIZE %s ... ", u->file);
+           logprintf (LOG_VERBOSE, "==> SIZE %s ... ", escnonprint (u->file));
        }
 
       err = ftp_size (csock, u->file, len);
@@ -760,7 +760,8 @@ Error in server response, closing control connection.\n"));
   if (restval && (cmd & DO_RETR))
     {
       if (!opt.server_response)
-       logprintf (LOG_VERBOSE, "==> REST %s ... ", number_to_static_string (restval));
+       logprintf (LOG_VERBOSE, "==> REST %s ... ",
+                  number_to_static_string (restval));
       err = ftp_rest (csock, restval);
 
       /* FTPRERR, WRITEFAILED, FTPRESTFAIL */
@@ -822,7 +823,7 @@ Error in server response, closing control connection.\n"));
            {
              if (restval)
                logputs (LOG_VERBOSE, "\n");
-             logprintf (LOG_VERBOSE, "==> RETR %s ... ", u->file);
+             logprintf (LOG_VERBOSE, "==> RETR %s ... ", escnonprint (u->file));
            }
        }
 
@@ -852,7 +853,8 @@ Error in server response, closing control connection.\n"));
          break;
        case FTPNSFOD:
          logputs (LOG_VERBOSE, "\n");
-         logprintf (LOG_NOTQUIET, _("No such file `%s'.\n\n"), u->file);
+         logprintf (LOG_NOTQUIET, _("No such file `%s'.\n\n"),
+                    escnonprint (u->file));
          fd_close (dtsock);
          fd_close (local_sock);
          return err;
@@ -1114,7 +1116,7 @@ Error in server response, closing control connection.\n"));
             no-buffering on opt.lfile.  */
          while ((line = read_whole_line (fp)))
            {
-             logprintf (LOG_ALWAYS, "%s\n", line);
+             logprintf (LOG_ALWAYS, "%s\n", escnonprint (line));
              xfree (line);
            }
          fclose (fp);
@@ -1536,19 +1538,18 @@ The sizes do not match (local %s) -- retrieving.\n\n"),
                            {
                              logprintf (LOG_VERBOSE, _("\
 Already have correct symlink %s -> %s\n\n"),
-                                        con->target, f->linkto);
+                                        con->target, escnonprint (f->linkto));
                               dlthis = 0;
                              break;
                            }
                        }
                    }
                  logprintf (LOG_VERBOSE, _("Creating symlink %s -> %s\n"),
-                            con->target, f->linkto);
+                            con->target, escnonprint (f->linkto));
                  /* Unlink before creating symlink!  */
                  unlink (con->target);
                  if (symlink (f->linkto, con->target) == -1)
-                   logprintf (LOG_NOTQUIET, "symlink: %s\n",
-                              strerror (errno));
+                   logprintf (LOG_NOTQUIET, "symlink: %s\n", strerror (errno));
                  logputs (LOG_VERBOSE, "\n");
                } /* have f->linkto */
 #else  /* not HAVE_SYMLINK */
@@ -1566,7 +1567,7 @@ Already have correct symlink %s -> %s\n\n"),
        case FT_DIRECTORY:
          if (!opt.recursive)
            logprintf (LOG_NOTQUIET, _("Skipping directory `%s'.\n"),
-                      f->name);
+                      escnonprint (f->name));
          break;
        case FT_PLAINFILE:
          /* Call the retrieve loop.  */
@@ -1575,7 +1576,7 @@ Already have correct symlink %s -> %s\n\n"),
          break;
        case FT_UNKNOWN:
          logprintf (LOG_NOTQUIET, _("%s: unknown/unsupported file type.\n"),
-                    f->name);
+                    escnonprint (f->name));
          break;
        }       /* switch */
 
@@ -1680,7 +1681,8 @@ ftp_retrieve_dirs (struct url *u, struct fileinfo *f, ccon *con)
       if (!accdir (newdir, ALLABS))
        {
          logprintf (LOG_VERBOSE, _("\
-Not descending to `%s' as it is excluded/not-included.\n"), newdir);
+Not descending to `%s' as it is excluded/not-included.\n"),
+                    escnonprint (newdir));
          continue;
        }
 
@@ -1743,7 +1745,8 @@ ftp_retrieve_glob (struct url *u, ccon *con, int action)
        {
          if (f->type != FT_DIRECTORY && !acceptable (f->name))
            {
-             logprintf (LOG_VERBOSE, _("Rejecting `%s'.\n"), f->name);
+             logprintf (LOG_VERBOSE, _("Rejecting `%s'.\n"),
+                        escnonprint (f->name));
              f = delelement (f, &start);
            }
          else
@@ -1756,7 +1759,8 @@ ftp_retrieve_glob (struct url *u, ccon *con, int action)
     {
       if (has_insecure_name_p (f->name))
        {
-         logprintf (LOG_VERBOSE, _("Rejecting `%s'.\n"), f->name);
+         logprintf (LOG_VERBOSE, _("Rejecting `%s'.\n"),
+                    escnonprint (f->name));
          f = delelement (f, &start);
        }
       else
@@ -1802,7 +1806,8 @@ ftp_retrieve_glob (struct url *u, ccon *con, int action)
          /* No luck.  */
          /* #### This message SUCKS.  We should see what was the
             reason that nothing was retrieved.  */
-         logprintf (LOG_VERBOSE, _("No matches on pattern `%s'.\n"), u->file);
+         logprintf (LOG_VERBOSE, _("No matches on pattern `%s'.\n"),
+                    escnonprint (u->file));
        }
       else /* GETONE or GETALL */
        {

diff --git a/src/host.c b/src/host.c
index 6f96845c63689008af0447e75349f22587bee44f..b550d786d3a05cfbff7cfcf8d1d6daaa77ff315a 100644 (file)
--- a/src/host.c
+++ b/src/host.c
@@ -723,7 +723,7 @@ lookup_host (const char *host, int flags)
   /* No luck with the cache; resolve HOST. */
 
   if (!silent && !numeric_address)
-    logprintf (LOG_VERBOSE, _("Resolving %s... "), host);
+    logprintf (LOG_VERBOSE, _("Resolving %s... "), escnonprint (host));
 
 #ifdef ENABLE_IPV6
   {

diff --git a/src/http.c b/src/http.c
index 6bc4f3be5be0a41994c7bbf57bf837b2ed8d9540..1afe586b3565fd4d73c581e586991d1aadbd1509 100644 (file)
--- a/src/http.c
+++ b/src/http.c
@@ -687,7 +687,7 @@ print_server_response_1 (const char *prefix, const char *b, const char *e)
   if (b < e && e[-1] == '\r')
     --e;
   BOUNDED_TO_ALLOCA (b, e, ln);
-  logprintf (LOG_VERBOSE, "%s%s\n", prefix, ln);
+  logprintf (LOG_VERBOSE, "%s%s\n", prefix, escnonprint (ln));
 }
 
 /* Print the server response, line by line, omitting the trailing CR
@@ -1306,7 +1306,7 @@ gethttp (struct url *u, struct http_stat *hs, int *dt, struct url *proxy)
          sock = pconn.socket;
          using_ssl = pconn.ssl;
          logprintf (LOG_VERBOSE, _("Reusing existing connection to %s:%d.\n"),
-                    pconn.host, pconn.port);
+                    escnonprint (pconn.host), pconn.port);
          DEBUGP (("Reusing fd %d.\n", sock));
        }
     }
@@ -1377,11 +1377,11 @@ gethttp (struct url *u, struct http_stat *hs, int *dt, struct url *proxy)
            {
            failed_tunnel:
              logprintf (LOG_NOTQUIET, _("Proxy tunneling failed: %s"),
-                        message ? message : "?");
+                        message ? escnonprint (message) : "?");
              xfree_null (message);
              return CONSSLERR;
            }
-         xfree (message);
+         xfree_null (message);
 
          /* SOCK is now *really* connected to u->host, so update CONN
             to reflect this.  That way register_persistent will
@@ -1458,7 +1458,8 @@ gethttp (struct url *u, struct http_stat *hs, int *dt, struct url *proxy)
   message = NULL;
   statcode = response_status (resp, &message);
   if (!opt.server_response)
-    logprintf (LOG_VERBOSE, "%2d %s\n", statcode, message ? message : "");
+    logprintf (LOG_VERBOSE, "%2d %s\n", statcode,
+              message ? escnonprint (message) : "");
   else
     {
       logprintf (LOG_VERBOSE, "\n");
@@ -1604,7 +1605,7 @@ gethttp (struct url *u, struct http_stat *hs, int *dt, struct url *proxy)
        {
          logprintf (LOG_VERBOSE,
                     _("Location: %s%s\n"),
-                    hs->newloc ? hs->newloc : _("unspecified"),
+                    hs->newloc ? escnonprint_uri (hs->newloc) : _("unspecified"),
                     hs->newloc ? _(" [following]") : "");
          if (keep_alive)
            skip_short_body (sock, contlen);
@@ -1691,7 +1692,7 @@ gethttp (struct url *u, struct http_stat *hs, int *dt, struct url *proxy)
            logputs (LOG_VERBOSE,
                     opt.ignore_length ? _("ignored") : _("unspecified"));
          if (type)
-           logprintf (LOG_VERBOSE, " [%s]\n", type);
+           logprintf (LOG_VERBOSE, " [%s]\n", escnonprint (type));
          else
            logputs (LOG_VERBOSE, "\n");
        }
@@ -2105,7 +2106,7 @@ File `%s' already there, will not retrieve.\n"), *hstat.local_file);
              xfree (hurl);
            }
          logprintf (LOG_NOTQUIET, _("%s ERROR %d: %s.\n"),
-                    tms, hstat.statcode, hstat.error);
+                    tms, hstat.statcode, escnonprint (hstat.error));
          logputs (LOG_VERBOSE, "\n");
          free_hstat (&hstat);
          xfree_null (dummy);
@@ -2190,7 +2191,8 @@ The sizes do not match (local %s) -- retrieving.\n"),
 
       if (opt.spider)
        {
-         logprintf (LOG_NOTQUIET, "%d %s\n\n", hstat.statcode, hstat.error);
+         logprintf (LOG_NOTQUIET, "%d %s\n\n", hstat.statcode,
+                    escnonprint (hstat.error));
          xfree_null (dummy);
          return RETROK;
        }

diff --git a/src/log.c b/src/log.c
index 2511513fc0ef95bb9ab0763d9a22499188508554..5477093fecbd67f318aef89273d3f9aa3801b2b7 100644 (file)
--- a/src/log.c
+++ b/src/log.c
@@ -615,6 +615,140 @@ log_dump_context (void)
   fflush (fp);
 }
 \f
+/* String escape functions. */
+
+/* Return the number of non-printable characters in SOURCE.
+
+   Non-printable characters are determined as per safe-ctype.h,
+   i.e. the non-printable characters of the "C" locale.  This code is
+   meant to be used to protect the user from binary characters in
+   (normally ASCII) server messages. */
+
+static int
+count_nonprint (const char *source)
+{
+  const char *p;
+  int cnt;
+  for (p = source, cnt = 0; *p; p++)
+    if (!ISPRINT (*p))
+      ++cnt;
+  return cnt;
+}
+
+/* Copy SOURCE to DEST, escaping non-printable characters.  If FOR_URI
+   is 0, they are escaped as \ooo; otherwise, they are escaped as
+   %xx.
+
+   DEST must point to a location with sufficient room to store an
+   encoded version of SOURCE.  */
+
+static void
+copy_and_escape (const char *source, char *dest, int for_uri)
+{
+  const char *from;
+  char *to;
+
+  /* Copy the string, escaping non-printable chars. */
+  if (!for_uri)
+    {
+      for (from = source, to = dest; *from; from++)
+       if (ISPRINT (*from))
+         *to++ = *from;
+       else
+         {
+           const unsigned char c = *from;
+           *to++ = '\\';
+           *to++ = '0' + (c >> 6);
+           *to++ = '0' + ((c >> 3) & 7);
+           *to++ = '0' + (c & 7);
+         }
+    }
+  else
+    {
+      for (from = source, to = dest; *from; from++)
+       if (ISPRINT (*from))
+         *to++ = *from;
+       else
+         {
+           const unsigned char c = *from;
+           *to++ = '%';
+           *to++ = XNUM_TO_DIGIT (c >> 4);
+           *to++ = XNUM_TO_DIGIT (c & 0xf);
+         }
+    }
+  *to = '\0';
+}
+
+#define RING_SIZE 3
+struct ringel {
+  char *buffer;
+  int size;
+};
+
+static const char *
+escnonprint_internal (const char *str, int for_uri)
+{
+  static struct ringel ring[RING_SIZE];        /* ring data */
+  static int ringpos;                  /* current ring position */
+
+  int nprcnt = count_nonprint (str);
+  if (nprcnt == 0)
+    /* If there are no non-printable chars in STR, don't bother
+       copying anything, just return STR.  */
+    return str;
+
+  {
+    /* Set up a pointer to the current ring position, so we can write
+       simply r->X instead of ring[ringpos].X. */
+    struct ringel *r = ring + ringpos;
+
+    /* Every non-printable character is replaced with "\ooo",
+       i.e. with three *additional* chars (two in URI-mode).  Size
+       must also include the length of the original string and an
+       additional char for the terminating \0. */
+    int needed_size = strlen (str) + 1 + for_uri ? (2 * nprcnt) : (3 * nprcnt);
+
+    /* If the current buffer is uninitialized or too small,
+       (re)allocate it.  */
+    if (r->buffer == NULL || r->size < needed_size)
+      r->buffer = xrealloc (r->buffer, needed_size);
+
+    copy_and_escape (str, r->buffer, for_uri);
+    ringpos = (ringpos + 1) % RING_SIZE;
+    return r->buffer;
+  }
+}
+
+/* Return a pointer to a static copy of STR with the non-printable
+   characters escaped as \ooo.  If there are no non-printable
+   characters in STR, STR is returned.
+
+   NOTE: since this function can return a pointer to static data, be
+   careful to copy its result before calling it again.  However, to be
+   more useful with printf, it maintains an internal ring of static
+   buffers to return.  Currently the ring size is 3, which means you
+   can print up to three values in the same printf; if more is needed,
+   bump RING_SIZE.  */
+
+const char *
+escnonprint (const char *str)
+{
+  return escnonprint_internal (str, 0);
+}
+
+/* Return a pointer to a static copy of STR with the non-printable
+   characters escaped as %XX.  If there are no non-printable
+   characters in STR, STR is returned.
+
+   This function returns a pointer to static data which will be
+   overwritten by subsequent calls -- see escnonprint for details.  */
+
+const char *
+escnonprint_uri (const char *str)
+{
+  return escnonprint_internal (str, 1);
+}
+\f
 /* When SIGHUP or SIGUSR1 are received, the output is redirected
    elsewhere.  Such redirection is only allowed once. */
 enum { RR_NONE, RR_REQUESTED, RR_DONE } redirect_request = RR_NONE;

diff --git a/src/log.h b/src/log.h
index 173f81e3d2acb3086c897b94451c4522c13fd5a2..d2961994557648dcaefbbf41ae21e4e100181bfd 100644 (file)
--- a/src/log.h
+++ b/src/log.h
@@ -52,4 +52,7 @@ void log_init PARAMS ((const char *, int));
 void log_close PARAMS ((void));
 void log_request_redirect_output PARAMS ((const char *));
 
+const char *escnonprint PARAMS ((const char *));
+const char *escnonprint_uri PARAMS ((const char *));
+
 #endif /* LOG_H */

diff --git a/src/retr.c b/src/retr.c
index 339a2fa0879c2418aff506cbd319ae0890381628..ef142bceb881330b03ee450f11a7b2b0b7bd6aff 100644 (file)
--- a/src/retr.c
+++ b/src/retr.c
@@ -699,7 +699,7 @@ retrieve_url (const char *origurl, char **file, char **newloc,
       newloc_parsed = url_parse (mynewloc, &up_error_code);
       if (!newloc_parsed)
        {
-         logprintf (LOG_NOTQUIET, "%s: %s.\n", mynewloc,
+         logprintf (LOG_NOTQUIET, "%s: %s.\n", escnonprint_uri (mynewloc),
                     url_error (up_error_code));
          url_free (u);
          xfree (url);

diff --git a/src/string_t.c b/src/string_t.c
index 90417a22f939ea09c12b77edcfb9e6f2e7b0a22e..b94f12037860fba3e9258542563ea9c600947fc8 100644 (file)
--- a/src/string_t.c
+++ b/src/string_t.c
@@ -31,9 +31,13 @@
 
 #include "config.h"
 
+#define _GNU_SOURCE            /* to get iswblank */
+
 #include <assert.h>
 #include <stdlib.h>
+#include <string.h>
 #include <wchar.h>
+#include <wctype.h>
 
 #include "wget.h"
 
@@ -182,12 +186,14 @@ string_destroy (struct string_t *str)
   memset (str, 0, sizeof (*str));
 }
 
+#if 0                          /* unused */
 static void
 string_append_delim (struct string_t *dst)
 {
   assert_valid_string (dst);
   string_cat (dst, line_delim, line_delim_len);
 }
+#endif
 
 static int 
 is_line_delim (const wchar_t *wsz)