[tor-commits] [tor/master] Validate SOCKS arguments.

[nickm at torproject.org]

commit faf4f6c6d1da54b0a6b0c9946112f2e448867a8f
Author: George Kadianakis <desnacked at riseup.net>
Date:   Mon Dec 17 14:39:24 2012 +0200

    Validate SOCKS arguments.
---
 src/or/config.c     |   39 +++++++++++++++++++++++++++++++++++++++
 src/or/connection.h |    8 ++++++++
 src/or/transports.c |   35 +++++++++++++++++++++++++++++++++++
 src/or/transports.h |    1 +
 4 files changed, 83 insertions(+), 0 deletions(-)

diff --git a/src/or/config.c b/src/or/config.c
index 3e75359..d057dd8 100644
--- a/src/or/config.c
+++ b/src/or/config.c
@@ -4025,6 +4025,40 @@ options_init_logs(or_options_t *options, int validate_only)
   return ok?0:-1;
 }
 
+/** Given a smartlist of SOCKS arguments to be passed to a transport
+ *  proxy in <b>args</b>, validate them and return -1 if they are
+ *  corrupted. Return 0 if they seem OK. */
+static int
+validate_transport_socks_arguments(const smartlist_t *args)
+{
+  char *socks_string = NULL;
+  size_t socks_string_len;
+
+  tor_assert(args);
+  tor_assert(smartlist_len(args) > 0);
+
+  SMARTLIST_FOREACH_BEGIN(args, const char *, s) {
+    if (!string_is_key_value(s)) /* arguments should  be k=v items */
+      return -1;
+  } SMARTLIST_FOREACH_END(s);
+
+  socks_string = pt_stringify_socks_args(args);
+  if (!socks_string)
+    return -1;
+
+  socks_string_len = strlen(socks_string);
+  tor_free(socks_string);
+
+  if (socks_string_len > MAX_SOCKS5_AUTH_SIZE_TOTAL) {
+    log_warn(LD_CONFIG, "SOCKS arguments can't be more than %u bytes (%lu).",
+             MAX_SOCKS5_AUTH_SIZE_TOTAL,
+             (unsigned long) socks_string_len);
+    return -1;
+  }
+
+  return 0;
+}
+
 /** Read the contents of a Bridge line from <b>line</b>. Return 0
  * if the line is well-formed, and -1 if it isn't. If
  * <b>validate_only</b> is 0, and the line is well-formed, then add
@@ -4143,6 +4177,11 @@ parse_bridge_line(const char *line, int validate_only)
     bridge_add_from_config(&addr, port,
                            fingerprint ? digest : NULL,
                            transport_name, socks_args);
+  } else {
+    if (socks_args) {
+      if (validate_transport_socks_arguments(socks_args) < 0)
+        goto err;
+    }
   }
 
   r = 0;
diff --git a/src/or/connection.h b/src/or/connection.h
index c78fe6e..3e656ec 100644
--- a/src/or/connection.h
+++ b/src/or/connection.h
@@ -89,6 +89,14 @@ int connection_connect(connection_t *conn, const char *address,
                        const tor_addr_t *addr,
                        uint16_t port, int *socket_error);
 
+/** Maximum size of information that we can fit into SOCKS5 username
+    or password fields. */
+#define MAX_SOCKS5_AUTH_FIELD_SIZE 255
+
+/** Total maximum size of information that we can fit into SOCKS5
+    username and password fields. */
+#define MAX_SOCKS5_AUTH_SIZE_TOTAL 2*MAX_SOCKS5_AUTH_FIELD_SIZE
+
 int connection_proxy_connect(connection_t *conn, int type);
 int connection_read_proxy_handshake(connection_t *conn);
 void log_failed_proxy_connection(connection_t *conn);
diff --git a/src/or/transports.c b/src/or/transports.c
index 647b293..a9f7fa2 100644
--- a/src/or/transports.c
+++ b/src/or/transports.c
@@ -1424,6 +1424,41 @@ pt_get_extra_info_descriptor_string(void)
   return the_string;
 }
 
+/** Stringify the SOCKS arguments in <b>socks_args</b> according to
+ *  180_pluggable_transport.txt.  The string is allocated on the heap
+ *  and it's the responsibility of the caller to free it after use. */
+char *
+pt_stringify_socks_args(const smartlist_t *socks_args)
+{
+  /* tmp place to store escaped socks arguments, so that we can
+     concatenate them up afterwards */
+  smartlist_t *sl_tmp = NULL;
+  char *escaped_string = NULL;
+  char *new_string = NULL;
+
+  tor_assert(socks_args);
+  tor_assert(smartlist_len(socks_args) > 0);
+
+  sl_tmp = smartlist_new();
+
+  SMARTLIST_FOREACH_BEGIN(socks_args, const char *, s) {
+    /* Escape ';' and '\'. */
+    escaped_string = tor_escape_str_for_socks_arg(s);
+    if (!escaped_string)
+      goto done;
+
+    smartlist_add(sl_tmp, escaped_string);
+  } SMARTLIST_FOREACH_END(s);
+
+  new_string = smartlist_join_strings(sl_tmp, ";", 0, NULL);
+
+ done:
+  SMARTLIST_FOREACH(sl_tmp, char *, s, tor_free(s));
+  smartlist_free(sl_tmp);
+
+  return new_string;
+}
+
 /** The tor config was read.
  *  Destroy all managed proxies that were marked by a previous call to
  *  prepare_proxy_list_for_config_read() and are not used by the new
diff --git a/src/or/transports.h b/src/or/transports.h
index 6ee82f4..16fd5d4 100644
--- a/src/or/transports.h
+++ b/src/or/transports.h
@@ -55,6 +55,7 @@ void pt_prepare_proxy_list_for_config_read(void);
 void sweep_proxy_list(void);
 
 smartlist_t *get_transport_proxy_ports(void);
+char *pt_stringify_socks_args(const smartlist_t *socks_args);
 
 #ifdef PT_PRIVATE
 /** State of the managed proxy configuration protocol. */