Add some functions to escape values from the network before sending them to the log. Use them everywhere except for routerinfo->plaftorm, routerinfo->contact_info, and rend*.c. (need sleep now)

svn:r6087

doc/TODO

@@ -45,9 +45,13 @@ N - building on freebsd 6.0: (with multiple openssl installations)
     - authorities should *never* 503 a cache, but *should* 503 clients
       when they feel like it.
     - update dir-spec with what we decided for each of these
-  - when logging unknown http headers, this could include bad escape codes?
-    - more generally, attacker-controller log entries with newlines in them
-      are dangerous for our users.
+  o when logging unknown http headers, this could include bad escape codes?
+    more generally, attacker-controller log entries with newlines in them
+    are dangerous for our users.
+    o So... add functions to escape potentially malicious values before
+      logging them, and test values more closely as they arrive...
+    - But what to do about contact_info and platform?
+    - (Didn't finish converting rend*.c)
   - Make "setconf" and "hup" behavior cleaner for LINELIST config
     options (e.g. Log). Bug 238.
 R - Jan 26 10:25:04.832 [warn] add_an_entry_guard(): Tried finding a
@@ -56,11 +60,11 @@ R - streamline how we define a guard node as 'up'. document it
     somewhere.
 R - reduce log severity for guard nodes.
 R - make guard node timeout higher.
-N . Clean and future-proof exit policy formats a bit.
+  o Clean and future-proof exit policy formats a bit.
     o Likewise accept, but don't generate /bits formats (unless they're
       accepted in 0.0.9 and later).
     o Warn when we see a netmask that isn't a prefix.
-    - Make clients understand "private:*" in exit policies, even though
+    o Make clients understand "private:*" in exit policies, even though
       we don't generate it yet.
 
 for 0.1.1.x-final:

src/common/tortls.c

@@ -672,7 +672,8 @@ tor_tls_get_peer_cert_nickname(tor_tls_t *tls, char *buf, size_t buflen)
     goto error;
   if (((int)strspn(buf, LEGAL_NICKNAME_CHARACTERS)) < lenout) {
     log_warn(LD_PROTOCOL,
-             "Peer certificate nickname \"%s\" has illegal characters.", buf);
+             "Peer certificate nickname %s has illegal characters.",
+             escaped(buf));
     if (strchr(buf, '.'))
       log_warn(LD_PROTOCOL, "  (Maybe it is not really running Tor at its "
                "advertised OR port.)");

src/common/util.c

@@ -329,6 +329,16 @@ tor_strupper(char *s)
   }
 }
 
+int
+tor_strisprint(const char *s)
+{
+  while (*s) {
+    if (!TOR_ISPRINT(*s))
+      return 0;
+  }
+  return 1;
+}
+
 /* Compares the first strlen(s2) characters of s1 with s2.  Returns as for
  * strcmp.
  */
@@ -564,6 +574,100 @@ base16_decode(char *dest, size_t destlen, const char *src, size_t srclen)
   return 0;
 }
 
+/** Allocate and return a new string representing the contents of <b>s</b>,
+ * surrounded by quotes and using standard C escapes.
+ *
+ * Generally, we use this for logging values that come in over the network
+ * to keep them from tricking users.
+ *
+ * We trust values from the resolver, OS, configuration file, and command line
+ * to not be maliciously ill-formed.  We validate incoming routerdescs and
+ * SOCKS requests and addresses from BEGIN cells as they're parsed;
+ * afterwards, we trust them as non-malicious.
+ */
+char *
+esc_for_log(const char *s)
+{
+  const char *cp;
+  char *result, *outp;
+  size_t len = 3;
+  for (cp = s; *cp; ++cp) {
+    switch (*cp) {
+      case '\\':
+      case '\"':
+      case '\'':
+        len += 2;
+        break;
+      default:
+        if (TOR_ISPRINT(*cp))
+          ++len;
+        else
+          len += 4;
+        break;
+    }
+  }
+
+  result = outp = tor_malloc(len);
+  *outp++ = '\"';
+  for (cp = s; *cp; ++cp) {
+    switch (*cp) {
+      case '\\':
+      case '\"':
+      case '\'':
+        *outp++ = '\\';
+        *outp++ = *cp;
+        break;
+      case '\n':
+        *outp++ = '\\';
+        *outp++ = 'n';
+        break;
+      case '\t':
+        *outp++ = '\\';
+        *outp++ = 't';
+        break;
+      case '\r':
+        *outp++ = '\\';
+        *outp++ = 'r';
+        break;
+      default:
+        if (TOR_ISPRINT(*cp)) {
+          *outp++ = *cp;
+        } else {
+          tor_snprintf(outp, 5, "\\%03o", (uint8_t) *cp);
+          outp += 4;
+        }
+        break;
+    }
+  }
+
+  *outp++ = '\"';
+  *outp++ = 0;
+
+  return result;
+}
+
+/** Allocate and return a new string representing the contents of <b>s</b>,
+ * surrounded by quotes and using standard C escapes.
+ *
+ * THIS FUNCTION IS NOT REENTRANT.  Don't call it from outside the main
+ * thread.  Also, each call invalidates the last-returned value, so don't
+ * try log_warn(LD_GENERAL, "%s %s", escaped(a), escaped(b));
+ */
+const char *
+escaped(const char *s)
+{
+  static char *_escaped_val = NULL;
+  if (_escaped_val)
+    tor_free(_escaped_val);
+
+  if (s)
+    _escaped_val = esc_for_log(s);
+  else
+    _escaped_val = NULL;
+
+  return _escaped_val;
+}
+
 /* =====
  * Time
  * ===== */
@@ -700,7 +804,9 @@ parse_rfc1123_time(const char *buf, time_t *t)
   if (sscanf(buf, "%3s, %d %3s %d %d:%d:%d GMT", weekday,
              &tm.tm_mday, month, &tm.tm_year, &tm.tm_hour,
              &tm.tm_min, &tm.tm_sec) < 7) {
-    log_warn(LD_GENERAL, "Got invalid RFC1123 time \"%s\"", buf);
+    char *esc = esc_for_log(buf);
+    log_warn(LD_GENERAL, "Got invalid RFC1123 time %s", esc);
+    tor_free(esc);
     return -1;
   }
 
@@ -712,14 +818,18 @@ parse_rfc1123_time(const char *buf, time_t *t)
     }
   }
   if (m<0) {
-    log_warn(LD_GENERAL, "Got invalid RFC1123 time \"%s\"", buf);
+    char *esc = esc_for_log(buf);
+    log_warn(LD_GENERAL, "Got invalid RFC1123 time %s", esc);
+    tor_free(esc);
     return -1;
   }
   tm.tm_mon = m;
 
   if (tm.tm_year < 1970) {
+    char *esc = esc_for_log(buf);
     log_warn(LD_GENERAL,
-             "Got invalid RFC1123 time \"%s\". (Before 1970)", buf);
+             "Got invalid RFC1123 time %s. (Before 1970)", esc);
+    tor_free(esc);
     return -1;
   }
   tm.tm_year -= 1900;
@@ -768,7 +878,9 @@ parse_iso_time(const char *cp, time_t *t)
   st_tm.tm_sec = second;
 #endif
   if (st_tm.tm_year < 70) {
-    log_warn(LD_GENERAL, "Got invalid ISO time \"%s\". (Before 1970)", cp);
+    char *esc = esc_for_log(cp);
+    log_warn(LD_GENERAL, "Got invalid ISO time %s. (Before 1970)", esc);
+    tor_free(esc);
     return -1;
   }
   *t = tor_timegm(&st_tm);
@@ -1222,7 +1334,7 @@ expand_filename(const char *filename)
       home = getenv("HOME");
       if (!home) {
         log_warn(LD_CONFIG, "Couldn't find $HOME environment variable while "
-                 "expanding %s", filename);
+                 "expanding \"%s\"", filename);
         return NULL;
       }
       home = tor_strdup(home);
@@ -1385,13 +1497,15 @@ parse_addr_port(const char *addrport, char **address, uint32_t *addr,
     _address = tor_strndup(addrport, colon-addrport);
     _port = (int) tor_parse_long(colon+1,10,1,65535,NULL,NULL);
     if (!_port) {
-      log_warn(LD_GENERAL, "Port '%s' out of range", colon+1);
+      log_warn(LD_GENERAL, "Port %s out of range", escaped(colon+1));
       ok = 0;
     }
     if (!port_out) {
+      char *esc_addrport = esc_for_log(addrport);
       log_warn(LD_GENERAL,
-               "Port '%s' given on '%s' when not required",
-               colon+1, addrport);
+               "Port %s given on %s when not required",
+               escaped(colon+1), esc_addrport);
+      tor_free(esc_addrport);
       ok = 0;
     }
   } else {
@@ -1402,7 +1516,7 @@ parse_addr_port(const char *addrport, char **address, uint32_t *addr,
   if (addr) {
     /* There's an addr pointer, so we need to resolve the hostname. */
     if (tor_lookup_hostname(_address,addr)) {
-      log_warn(LD_NET, "Couldn't look up '%s'", _address);
+      log_warn(LD_NET, "Couldn't look up %s", escaped(_address));
       ok = 0;
       *addr = 0;
     }
@@ -1464,13 +1578,13 @@ parse_port_range(const char *port, uint16_t *port_min_out,
                                                 &endptr);
       if (*endptr || !*port_max_out) {
         log_warn(LD_GENERAL,
-                 "Malformed port \"%s\" on address range rejecting.",
-                 port);
+                 "Malformed port %s on address range rejecting.",
+                 escaped(port));
       }
     } else if (*endptr || !*port_min_out) {
       log_warn(LD_GENERAL,
-               "Malformed port \"%s\" on address range; rejecting.",
-               port);
+               "Malformed port %s on address range; rejecting.",
+               escaped(port));
       return -1;
     } else {
       *port_max_out = *port_min_out;
@@ -1523,8 +1637,8 @@ parse_addr_and_port_range(const char *s, uint32_t *addr_out,
   } else if (tor_inet_aton(address, &in) != 0) {
     *addr_out = ntohl(in.s_addr);
   } else {
-    log_warn(LD_GENERAL, "Malformed IP \"%s\" in address pattern; rejecting.",
-             address);
+    log_warn(LD_GENERAL, "Malformed IP %s in address pattern; rejecting.",
+             escaped(address));
     goto err;
   }
 
@@ -1548,8 +1662,8 @@ parse_addr_and_port_range(const char *s, uint32_t *addr_out,
       *mask_out = ntohl(in.s_addr);
     } else {
       log_warn(LD_GENERAL,
-               "Malformed mask \"%s\" on address range; rejecting.",
-               mask);
+               "Malformed mask %s on address range; rejecting.",
+               escaped(mask));
       goto err;
     }
   }

src/common/util.h

@@ -90,6 +90,7 @@ extern int dmalloc_free(const char *file, const int line, void *pnt,
 #define HEX_CHARACTERS "0123456789ABCDEFabcdef"
 void tor_strlower(char *s);
 void tor_strupper(char *s);
+int tor_strisprint(const char *s);
 int strcmpstart(const char *s1, const char *s2);
 int strcasecmpstart(const char *s1, const char *s2);
 int strcmpend(const char *s1, const char *s2);
@@ -112,6 +113,8 @@ const char *eat_whitespace(const char *s);
 const char *eat_whitespace_no_nl(const char *s);
 const char *find_whitespace(const char *s);
 int tor_digest_is_zero(const char *digest);
+char *esc_for_log(const char *string);
+const char *escaped(const char *string);
 
 void base16_encode(char *dest, size_t destlen, const char *src, size_t srclen);
 int base16_decode(char *dest, size_t destlen, const char *src, size_t srclen);

src/or/buffers.c

@@ -1004,6 +1004,14 @@ fetch_from_buf_socks(buf_t *buf, socks_request_t *req, int log_sockstype)
           req->address[len] = 0;
           req->port = ntohs(get_uint16(buf->cur+5+len));
           buf_remove_from_front(buf, 5+len+2);
+          if (!tor_strisprint(req->address) || strchr(req->address,'\"')) {
+            log_warn(LD_PROTOCOL,
+                     "Your application (using socks5 on port %d) gave Tor "
+                     "a malformed hostname: %s. Rejecting the connection.",
+                     req->port, escaped(req->address));
+            return -1;
+          }
+
           if (log_sockstype)
             log_notice(LD_APP,
                   "Your application (using socks5 on port %d) gave "
@@ -1049,7 +1057,7 @@ fetch_from_buf_socks(buf_t *buf, socks_request_t *req, int log_sockstype)
           return -1;
         }
         log_debug(LD_APP,
-                  "socks4: successfully read destip (%s)",safe_str(tmpbuf));
+                  "socks4: successfully read destip (%s)", safe_str(tmpbuf));
         socks4_prot = socks4;
       }
 
@@ -1088,6 +1096,7 @@ fetch_from_buf_socks(buf_t *buf, socks_request_t *req, int log_sockstype)
           return -1;
         }
         tor_assert(next < buf->cur+buf->datalen);
+
         if (log_sockstype)
           log_notice(LD_APP,
                      "Your application (using socks4a on port %d) gave "
@@ -1097,6 +1106,13 @@ fetch_from_buf_socks(buf_t *buf, socks_request_t *req, int log_sockstype)
       log_debug(LD_APP,"socks4: Everything is here. Success.");
       strlcpy(req->address, startaddr ? startaddr : tmpbuf,
               sizeof(req->address));
+      if (!tor_strisprint(req->address) || strchr(req->address,'\"')) {
+        log_warn(LD_PROTOCOL,
+                 "Your application (using socks4 on port %d) gave Tor "
+                 "a malformed hostname: %s. Rejecting the connection.",
+                 req->port, escaped(req->address));
+        return -1;
+      }
       /* next points to the final \0 on inbuf */
       buf_remove_from_front(buf, next-buf->cur+1);
       return 1;

src/or/circuituse.c

@@ -733,7 +733,7 @@ circuit_build_failed(circuit_t *circ)
       log_info(LD_REND,
                "Couldn't connect to Alice's chosen rend point %s "
                "(%s hop failed).",
-               build_state_get_exit_nickname(circ->build_state),
+               escaped(build_state_get_exit_nickname(circ->build_state)),
                failed_at_last_hop?"last":"non-last");
       rend_service_relaunch_rendezvous(circ);
       break;

src/or/command.c

@@ -202,8 +202,8 @@ command_process_create_cell(cell_t *cell, connection_t *conn)
            cell->circ_id, (int)(time(NULL) - conn->timestamp_created));
     if (router)
       log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL,
-             "Details: nickname '%s', platform '%s'.",
-             router->nickname, router->platform);
+             "Details: nickname \"%s\", platform %s.",
+             router->nickname, escaped(router->platform));
     return;
   }
 

src/or/config.c

@@ -518,6 +518,16 @@ safe_str(const char *address)
     return address;
 }
 
+/** DOCDOC */
+const char *
+escaped_safe_str(const char *address)
+{
+  if (get_options()->SafeLogging)
+    return "[scrubbed]";
+  else
+    return escaped(address);
+}
+
 /** Add the default directory servers directly into the trusted dir list. */
 static void
 add_default_trusted_dirservers(void)

src/or/connection.c

@@ -772,7 +772,7 @@ connection_connect(connection_t *conn, char *address,
   } else if (!SOCKET_IS_POLLABLE(s)) {
     log_warn(LD_NET,
             "Too many connections; can't create pollable connection to %s",
-            safe_str(address));
+             safe_str(address));
     tor_close_socket(s);
     return -1;
   }

src/or/connection_edge.c

@@ -945,7 +945,8 @@ connection_ap_handshake_rewrite_and_attach(connection_t *conn)
   hostname_type_t addresstype;
 
   tor_strlower(socks->address); /* normalize it */
-  log_debug(LD_APP,"Client asked for %s:%d", safe_str(socks->address),
+  log_debug(LD_APP,"Client asked for %s:%d",
+            safe_str(socks->address),
             socks->port);
 
   /* For address map controls, remap the address */
@@ -1519,6 +1520,12 @@ connection_exit_begin_conn(cell_t *cell, circuit_t *circ)
     tor_free(address);
     return 0;
   }
+  if (!tor_strisprint(address)) {
+    log_warn(LD_PROTOCOL,"Non-printing characters in address %s in relay "
+             "begin cell. Dropping.", escaped(address));
+    tor_free(address);
+    return 0;
+  }
 
   log_debug(LD_EXIT,"Creating new exit connection.");
   n_stream = connection_new(CONN_TYPE_EXIT);

src/or/connection_or.c

@@ -181,8 +181,8 @@ connection_or_read_proxy_response(connection_t *conn)
 
   if (status_code == 200) {
     log_info(LD_OR,
-             "HTTPS connect to '%s' successful! (200 \"%s\") Starting TLS.",
-             conn->address, reason);
+             "HTTPS connect to '%s' successful! (200 %s) Starting TLS.",
+             conn->address, escaped(reason));
     tor_free(reason);
     if (connection_tls_start_handshake(conn, 0) < 0) {
       /* TLS handshaking error of some kind. */
@@ -194,9 +194,9 @@ connection_or_read_proxy_response(connection_t *conn)
   }
   /* else, bad news on the status code */
   log_warn(LD_OR,
-           "The https proxy sent back an unexpected status code %d (\"%s\"). "
+           "The https proxy sent back an unexpected status code %d (%s). "
            "Closing.",
-           status_code, reason);
+           status_code, escaped(reason));
   tor_free(reason);
   connection_mark_for_close(conn);
   return -1;

src/or/directory.c

@@ -537,7 +537,7 @@ directory_send_command(connection_t *conn, const char *platform,
       tor_assert(!payload);
       log_debug(LD_DIR,
                 "Asking for compressed directory from server running %s",
-                platform?platform:"<unknown version>");
+                platform?escaped(platform):"<unknown version>");
       httpcommand = "GET";
       url = tor_strdup("/tor/dir.z");
       break;
@@ -712,7 +712,8 @@ http_get_origin(const char *headers, connection_t *conn)
   if (fwd) {
     size_t len = strlen(fwd)+strlen(conn->address)+32;
     char *result = tor_malloc(len);
-    tor_snprintf(result, len, "'%s' (forwarded for '%s')", conn->address, fwd);
+    tor_snprintf(result, len, "'%s' (forwarded for %s)", conn->address,
+                 escaped(fwd));
     tor_free(fwd);
     return result;
   } else {
@@ -754,7 +755,7 @@ parse_http_response(const char *headers, int *code, time_t *date,
   if (sscanf(headers, "HTTP/1.%d %d", &n1, &n2) < 2 ||
       (n1 != 0 && n1 != 1) ||
       (n2 < 100 || n2 >= 600)) {
-    log_warn(LD_HTTP,"Failed to parse header '%s'",headers);
+    log_warn(LD_HTTP,"Failed to parse header %s",escaped(headers));
     return -1;
   }
   *code = n2;
@@ -801,8 +802,8 @@ parse_http_response(const char *headers, int *code, time_t *date,
     } else if (!strcmp(enc, "gzip") || !strcmp(enc, "x-gzip")) {
       *compression = GZIP_METHOD;
     } else {
-      log_info(LD_HTTP, "Unrecognized content encoding: '%s'. Trying to deal.",
-               enc);
+      log_info(LD_HTTP, "Unrecognized content encoding: %s. Trying to deal.",
+               escaped(enc));
       *compression = -1;
     }
   }
@@ -891,8 +892,8 @@ connection_dir_client_reached_eof(connection_t *conn)
   if (!reason) reason = tor_strdup("[no reason given]");
 
   log_debug(LD_DIR,
-            "Received response from directory server '%s:%d': %d \"%s\"",
-            conn->address, conn->port, status_code, reason);
+            "Received response from directory server '%s:%d': %d %s",
+            conn->address, conn->port, status_code, escaped(reason));
 
   if (date_header > 0) {
     now = time(NULL);
@@ -914,9 +915,9 @@ connection_dir_client_reached_eof(connection_t *conn)
   }
 
   if (status_code == 503) {
-    log_info(LD_DIR,"Received http status code %d (\"%s\") from server "
+    log_info(LD_DIR,"Received http status code %d (%s) from server "
              "'%s:%d'. I'll try again soon.",
-             status_code, reason, conn->address, conn->port);
+             status_code, escaped(reason), conn->address, conn->port);
     tor_free(body); tor_free(headers); tor_free(reason);
     return -1;
   }
@@ -982,9 +983,9 @@ connection_dir_client_reached_eof(connection_t *conn)
     log_info(LD_DIR,"Received directory (size %d) from server '%s:%d'",
              (int)body_len, conn->address, conn->port);
     if (status_code != 200) {
-      log_warn(LD_DIR,"Received http status code %d (\"%s\") from server "
+      log_warn(LD_DIR,"Received http status code %d (%s) from server "
                "'%s:%d'. I'll try again soon.",
-               status_code, reason, conn->address, conn->port);
+               status_code, escaped(reason), conn->address, conn->port);
       tor_free(body); tor_free(headers); tor_free(reason);
       return -1;
     }
@@ -999,9 +1000,9 @@ connection_dir_client_reached_eof(connection_t *conn)
     /* just update our list of running routers, if this list is new info */
     log_info(LD_DIR,"Received running-routers list (size %d)", (int)body_len);
     if (status_code != 200) {
-      log_warn(LD_DIR,"Received http status code %d (\"%s\") from server "
+      log_warn(LD_DIR,"Received http status code %d (%s) from server "
                "'%s:%d'. I'll try again soon.",
-               status_code, reason, conn->address, conn->port);
+               status_code, escaped(reason), conn->address, conn->port);
       tor_free(body); tor_free(headers); tor_free(reason);
       return -1;
     }
@@ -1023,9 +1024,9 @@ connection_dir_client_reached_eof(connection_t *conn)
              "'%s:%d'",(int) body_len, conn->address, conn->port);
     if (status_code != 200) {
       log_warn(LD_DIR,
-           "Received http status code %d (\"%s\") from server "
+           "Received http status code %d (%s) from server "
            "'%s:%d' while fetching \"/tor/status/%s\". I'll try again soon.",
-           status_code, reason, conn->address, conn->port,
+           status_code, escaped(reason), conn->address, conn->port,
            conn->requested_resource);
       tor_free(body); tor_free(headers); tor_free(reason);
       connection_dir_download_networkstatus_failed(conn);
@@ -1083,9 +1084,9 @@ connection_dir_client_reached_eof(connection_t *conn)
       /* 404 means that it didn't have them; no big deal.
        * Older (pre-0.1.1.8) servers said 400 Servers unavailable instead. */
       log_fn(dir_okay ? LOG_INFO : LOG_WARN, LD_DIR,
-             "Received http status code %d (\"%s\") from server '%s:%d' "
+             "Received http status code %d (%s) from server '%s:%d' "
              "while fetching \"/tor/server/%s\". I'll try again soon.",
-             status_code, reason, conn->address, conn->port,
+             status_code, escaped(reason), conn->address, conn->port,
              conn->requested_resource);
       if (!which) {
         connection_dir_download_routerdesc_failed(conn);
@@ -1137,22 +1138,22 @@ connection_dir_client_reached_eof(connection_t *conn)
                  "descriptor: finished.");
         break;
       case 400:
-        log_warn(LD_GENERAL,"http status 400 (\"%s\") response from "
+        log_warn(LD_GENERAL,"http status 400 (%s) response from "
                  "dirserver '%s:%d'. Please correct.",
-                 reason, conn->address, conn->port);
+                 escaped(reason), conn->address, conn->port);
         break;
       case 403:
         log_warn(LD_GENERAL,
-             "http status 403 (\"%s\") response from dirserver "
+             "http status 403 (%s) response from dirserver "
              "'%s:%d'. Is your clock skewed? Have you mailed us your key "
              "fingerprint? Are you using the right key? Are you using a "
              "private IP address? See http://tor.eff.org/doc/"
-             "tor-doc-server.html", reason, conn->address, conn->port);
+             "tor-doc-server.html",escaped(reason), conn->address, conn->port);
         break;
       default:
         log_warn(LD_GENERAL,
-             "http status %d (\"%s\") reason unexpected (server '%s:%d').",
-             status_code, reason, conn->address, conn->port);
+             "http status %d (%s) reason unexpected (server '%s:%d').",
+             status_code, escaped(reason), conn->address, conn->port);
         break;
     }
     /* return 0 in all cases, since we don't want to mark any
@@ -1161,8 +1162,8 @@ connection_dir_client_reached_eof(connection_t *conn)
 
   if (conn->purpose == DIR_PURPOSE_FETCH_RENDDESC) {
     log_info(LD_REND,"Received rendezvous descriptor (size %d, status %d "
-             "(\"%s\"))",
-             (int)body_len, status_code, reason);
+             "(%s))",
+             (int)body_len, status_code, escaped(reason));
     switch (status_code) {
       case 200:
         if (rend_cache_store(body, body_len) < 0) {
@@ -1181,13 +1182,13 @@ connection_dir_client_reached_eof(connection_t *conn)
         break;
       case 400:
         log_warn(LD_REND,
-                 "http status 400 (\"%s\"). Dirserver didn't like our "
-                 "rendezvous query?", reason);
+                 "http status 400 (%s). Dirserver didn't like our "
+                 "rendezvous query?", escaped(reason));
         break;
       default:
-        log_warn(LD_REND,"http status %d (\"%s\") response unexpected (server "
-             "'%s:%d').",
-             status_code, reason, conn->address, conn->port);
+        log_warn(LD_REND,"http status %d (%s) response unexpected (server "
+                 "'%s:%d').",
+                 status_code, escaped(reason), conn->address, conn->port);
         break;
     }
   }
@@ -1197,17 +1198,17 @@ connection_dir_client_reached_eof(connection_t *conn)
       case 200:
         log_info(LD_REND,
                  "Uploading rendezvous descriptor: finished with status "
-                 "200 (\"%s\")", reason);
+                 "200 (%s)", escaped(reason));
         break;
       case 400:
-        log_warn(LD_REND,"http status 400 (\"%s\") response from dirserver "
+        log_warn(LD_REND,"http status 400 (%s) response from dirserver "
                  "'%s:%d'. Malformed rendezvous descriptor?",
-                 reason, conn->address, conn->port);
+                 escaped(reason), conn->address, conn->port);
         break;
       default:
-        log_warn(LD_REND,"http status %d (\"%s\") response unexpected (server "
+        log_warn(LD_REND,"http status %d (%s) response unexpected (server "
                  "'%s:%d').",
-                 status_code, reason, conn->address, conn->port);
+                 status_code, escaped(reason), conn->address, conn->port);
         break;
     }
   }
@@ -1728,7 +1729,7 @@ directory_handle_command_post(connection_t *conn, char *headers,
 #if 0
       if (body_len <= 1024) {
         base16_encode(tmp, sizeof(tmp), body, body_len);
-        log_notice(LD_DIRSERV,"Body was: %s", tmp);
+        log_notice(LD_DIRSERV,"Body was: %s", escaped(tmp));
       }
 #endif
       write_http_status_line(conn, 400, "Invalid service descriptor rejected");
@@ -1775,15 +1776,15 @@ directory_handle_command(connection_t *conn)
     /* case 1, fall through */
   }
 
-  log_debug(LD_DIRSERV,"headers '%s', body '%s'.", headers, body);
+  //log_debug(LD_DIRSERV,"headers %s, body %s.", headers, body);
 
   if (!strncasecmp(headers,"GET",3))
     r = directory_handle_command_get(conn, headers, body, body_len);
   else if (!strncasecmp(headers,"POST",4))
     r = directory_handle_command_post(conn, headers, body, body_len);
   else {
-    log_warn(LD_PROTOCOL,"Got headers '%s' with unknown command. Closing.",
-             headers);
+    log_warn(LD_PROTOCOL,"Got headers %s with unknown command. Closing.",
+             escaped(headers));
     r = -1;
   }
 
@@ -1904,7 +1905,7 @@ dir_routerdesc_download_failed(smartlist_t *failed)
   /* update_router_descriptor_downloads(time(NULL)); */
 }
 
-/* Given a directory <b>resource</b> request generated by us, containing zero
+/* Given a directory <b>resource</b> request, containing zero
  * or more strings separated by plus signs, followed optionally by ".z", store
  * the strings, in order, into <b>fp_out</b>.  If <b>compressed_out</b> is
  * non-NULL, set it to 1 if the resource ends in ".z", else set it to 0.  If
@@ -1938,13 +1939,13 @@ dir_split_resource_into_fingerprints(const char *resource,
       cp = smartlist_get(fp_out, i);
       if (strlen(cp) != HEX_DIGEST_LEN) {
         log_info(LD_DIR,
-                 "Skipping digest \"%s\" with non-standard length.", cp);
+                 "Skipping digest %s with non-standard length.", escaped(cp));
         smartlist_del(fp_out, i--);
         goto again;
       }
       d = tor_malloc_zero(DIGEST_LEN);
       if (base16_decode(d, DIGEST_LEN, cp, HEX_DIGEST_LEN)<0) {
-        log_info(LD_DIR, "Skipping non-decodable digest \"%s\"", cp);
+        log_info(LD_DIR, "Skipping non-decodable digest %s", escaped(cp));
         smartlist_del(fp_out, i--);
         goto again;
       }

src/or/dirserv.c

@@ -346,13 +346,14 @@ dirserv_get_status_impl(const char *fp, const char *nickname,
       log_debug(LD_DIRSERV,"Good fingerprint for '%s'",nickname);
     return FP_NAMED; /* Right fingerprint. */
   } else {
-    if (should_log)
+    if (should_log) {
       log_warn(LD_DIRSERV,
                "Mismatched fingerprint for '%s': expected '%s' got '%s'. "
                "ContactInfo '%s', platform '%s'.)",
                nickname, nn_ent->fingerprint, fp,
                contact ? contact : "",
-               platform ? platform : "");
+               platform ? escaped(platform) : "");
+    }
     if (msg)
       *msg = "Rejected: There is already a verified server with this nickname "
         "and a different fingerprint.";

src/or/or.h

@@ -1591,6 +1591,7 @@ or_options_t *get_options(void);
 int set_options(or_options_t *new_val);
 void config_free_all(void);
 const char *safe_str(const char *address);
+const char *escaped_safe_str(const char *address);
 
 int config_get_lines(char *string, config_line_t **result);
 void config_free_lines(config_line_t *front);

src/or/routerparse.c

@@ -468,8 +468,8 @@ router_parse_runningrouters(const char *str)
     log_warn(LD_DIR, "Error tokenizing directory"); goto err;
   }
   if ((tok = find_first_by_keyword(tokens, _UNRECOGNIZED))) {
-    log_warn(LD_DIR, "Unrecognized keyword '%s'; can't parse running-routers",
-             tok->args[0]);
+    log_warn(LD_DIR, "Unrecognized keyword %s; can't parse running-routers",
+             escaped(tok->args[0]));
     goto err;
   }
   tok = smartlist_get(tokens,0);
@@ -736,9 +736,9 @@ router_parse_entry_from_string(const char *s, const char *end)
   }
   if ((tok = find_first_by_keyword(tokens, _UNRECOGNIZED))) {
     log_warn(LD_DIR,
-             "Unrecognized critical keyword '%s'; skipping descriptor. "
+             "Unrecognized critical keyword %s; skipping descriptor. "
              "(It may be from another version of Tor.)",
-             tok->args[0]);
+             escaped(tok->args[0]));
     goto err;
   }
 
@@ -852,7 +852,8 @@ router_parse_entry_from_string(const char *s, const char *end)
     }
     tor_strstrip(tok->args[0], " ");
     if (base16_decode(d, DIGEST_LEN, tok->args[0], strlen(tok->args[0]))) {
-      log_warn(LD_DIR, "Couldn't decode fingerprint '%s'", tok->args[0]);
+      log_warn(LD_DIR, "Couldn't decode fingerprint %s",
+               escaped(tok->args[0]));
       goto err;
     }
     if (memcmp(d,router->cache_info.identity_digest, DIGEST_LEN)!=0) {
@@ -882,7 +883,8 @@ router_parse_entry_from_string(const char *s, const char *end)
     router->declared_family = smartlist_create();
     for (i=0;i<tok->n_args;++i) {
       if (!is_legal_nickname_or_hexdigest(tok->args[i])) {
-        log_warn(LD_DIR, "Illegal nickname '%s' in family line", tok->args[i]);
+        log_warn(LD_DIR, "Illegal nickname %s in family line",
+                 escaped(tok->args[i]));
         goto err;
       }
       smartlist_add(router->declared_family, tor_strdup(tok->args[i]));
@@ -977,8 +979,8 @@ routerstatus_parse_entry_from_string(const char **s, smartlist_t *tokens)
     goto err;
   }
   if ((tok = find_first_by_keyword(tokens, _UNRECOGNIZED))) {
-    log_warn(LD_DIR, "Unrecognized keyword \"%s\" in router status; skipping.",
-             tok->args[0]);
+    log_warn(LD_DIR, "Unrecognized keyword %s in router status; skipping.",
+             escaped(tok->args[0]));
     goto err;
   }
   if (!(tok = find_first_by_keyword(tokens, K_R))) {
@@ -993,19 +995,19 @@ routerstatus_parse_entry_from_string(const char **s, smartlist_t *tokens)
 
   if (!is_legal_nickname(tok->args[0])) {
     log_warn(LD_DIR,
-             "Invalid nickname '%s' in router status; skipping.",
-             tok->args[0]);
+             "Invalid nickname %s in router status; skipping.",
+             escaped(tok->args[0]));
     goto err;
   }
   strlcpy(rs->nickname, tok->args[0], sizeof(rs->nickname));
 
   if (digest_from_base64(rs->identity_digest, tok->args[1])) {
-    log_warn(LD_DIR, "Error decoding digest '%s'", tok->args[1]);
+    log_warn(LD_DIR, "Error decoding digest %s", escaped(tok->args[1]));
     goto err;
   }
 
   if (digest_from_base64(rs->descriptor_digest, tok->args[2])) {
-    log_warn(LD_DIR, "Error decoding digest '%s'", tok->args[2]);
+    log_warn(LD_DIR, "Error decoding digest %s", escaped(tok->args[2]));
     goto err;
   }
 
@@ -1018,7 +1020,7 @@ routerstatus_parse_entry_from_string(const char **s, smartlist_t *tokens)
   }
 
   if (tor_inet_aton(tok->args[5], &in) == 0) {
-    log_warn(LD_DIR, "Error parsing address '%s'", tok->args[5]);
+    log_warn(LD_DIR, "Error parsing address '%s'", escaped(tok->args[5]));
     goto err;
   }
   rs->addr = ntohl(in.s_addr);
@@ -1097,8 +1099,8 @@ networkstatus_parse_from_string(const char *s)
     goto err;
   }
   if ((tok = find_first_by_keyword(tokens, _UNRECOGNIZED))) {
-    log_warn(LD_DIR, "Unrecognized keyword '%s'; can't parse network-status",
-             tok->args[0]);
+    log_warn(LD_DIR, "Unrecognized keyword %s; can't parse network-status",
+             escaped(tok->args[0]));
     goto err;
   }
   ns = tor_malloc_zero(sizeof(networkstatus_t));
@@ -1119,7 +1121,7 @@ networkstatus_parse_from_string(const char *s)
   }
   ns->source_address = tok->args[0]; tok->args[0] = NULL;
   if (tor_inet_aton(tok->args[1], &in) == 0) {
-    log_warn(LD_DIR, "Error parsing address '%s'", tok->args[1]);
+    log_warn(LD_DIR, "Error parsing address %s", escaped(tok->args[1]));
     goto err;
   }
   ns->source_addr = ntohl(in.s_addr);
@@ -1140,7 +1142,7 @@ networkstatus_parse_from_string(const char *s)
   }
   if (base16_decode(ns->identity_digest, DIGEST_LEN, tok->args[0],
                     strlen(tok->args[0]))) {
-    log_warn(LD_DIR, "Couldn't decode fingerprint '%s'", tok->args[0]);
+    log_warn(LD_DIR, "Couldn't decode fingerprint %s", escaped(tok->args[0]));
     goto err;
   }
 
@@ -1369,7 +1371,7 @@ router_parse_addr_policy(directory_token_t *tok)
 
 policy_read_failed:
   tor_assert(newe->string);
-  log_warn(LD_DIR,"Couldn't parse line '%s'. Dropping", newe->string);
+  log_warn(LD_DIR,"Couldn't parse line %s. Dropping", escaped(newe->string));
   tor_free(newe->string);
   tor_free(newe);
   return NULL;

src/or/test.c

@@ -913,6 +913,12 @@ test_util(void)
     test_streq(tmpbuf, "18.244.0.188");
   }
 
+  /* Test 'escaped' */
+  test_streq("\"\"", escaped(""));
+  test_streq("\"abcd\"", escaped("abcd"));
+  test_streq("\"\\\\\\n\\r\\t\\\"\\'\"", escaped("\\\n\r\t\"\'"));
+  test_streq("\"z\\001abc\\277d\"", escaped("z\001abc\277d"));
+
   /* XXXX test older functions. */
   smartlist_free(sl);
 }