commit faf4f6c6d1da54b0a6b0c9946112f2e448867a8f Author: George Kadianakis desnacked@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. */