Merge branch 'bug13315_squashed'

Conflicts:
	src/or/buffers.c

changes/bug13315

@@ -0,0 +1,5 @@
+  o Minor features:
+    - Validate hostnames in SOCKS5 requests more strictly. If SafeSocks
+      is enabled, reject requests with IP addresses as hostnames. Resolves
+      ticket 13315.
+

src/common/util.c

@@ -964,6 +964,68 @@ string_is_key_value(int severity, const char *string)
   return 1;
 }
 
+/** Return true if <b>string</b> represents a valid IPv4 adddress in
+ * 'a.b.c.d' form.
+ */
+int
+string_is_valid_ipv4_address(const char *string)
+{
+  struct in_addr addr;
+
+  return (tor_inet_pton(AF_INET,string,&addr) == 1);
+}
+
+/** Return true if <b>string</b> represents a valid IPv6 address in
+ * a form that inet_pton() can parse.
+ */
+int
+string_is_valid_ipv6_address(const char *string)
+{
+  struct in6_addr addr;
+
+  return (tor_inet_pton(AF_INET6,string,&addr) == 1);
+}
+
+/** Return true iff <b>string</b> matches a pattern of DNS names
+ * that we allow Tor clients to connect to.
+ */
+int
+string_is_valid_hostname(const char *string)
+{
+  int result = 1;
+  smartlist_t *components;
+
+  components = smartlist_new();
+
+  smartlist_split_string(components,string,".",0,0);
+
+  SMARTLIST_FOREACH_BEGIN(components, char *, c) {
+    if (c[0] == '-') {
+      result = 0;
+      break;
+    }
+
+    do {
+      if ((*c >= 'a' && *c <= 'z') ||
+          (*c >= 'A' && *c <= 'Z') ||
+          (*c >= '0' && *c <= '9') ||
+          (*c == '-'))
+        c++;
+      else
+        result = 0;
+    } while (result && *c);
+
+  } SMARTLIST_FOREACH_END(c);
+
+  SMARTLIST_FOREACH_BEGIN(components, char *, c) {
+    tor_free(c);
+  } SMARTLIST_FOREACH_END(c);
+
+  smartlist_free(components);
+
+  return result;
+}
+
 /** Return true iff the DIGEST256_LEN bytes in digest are all zero. */
 int
 tor_digest256_is_zero(const char *digest)

src/common/util.h

@@ -227,6 +227,9 @@ const char *find_str_at_start_of_line(const char *haystack,
                                       const char *needle);
 int string_is_C_identifier(const char *string);
 int string_is_key_value(int severity, const char *string);
+int string_is_valid_hostname(const char *string);
+int string_is_valid_ipv4_address(const char *string);
+int string_is_valid_ipv6_address(const char *string);
 
 int tor_mem_is_zero(const char *mem, size_t len);
 int tor_digest_is_zero(const char *digest);

src/or/buffers.c

@@ -2054,8 +2054,20 @@ parse_socks(const char *data, size_t datalen, socks_request_t *req,
           req->address[len] = 0;
           req->port = ntohs(get_uint16(data+5+len));
           *drain_out = 5+len+2;
-          if (!tor_strisprint(req->address) || strchr(req->address,'\"')) {
+
+          if (string_is_valid_ipv4_address(req->address) ||
+              string_is_valid_ipv6_address(req->address)) {
+            log_unsafe_socks_warning(5,req->address,req->port,safe_socks);
+
+            if (safe_socks) {
+              socks_request_set_socks5_error(req, SOCKS5_NOT_ALLOWED);
+              return -1;
+            }
+          }
+
+          if (!string_is_valid_hostname(req->address)) {
             socks_request_set_socks5_error(req, SOCKS5_GENERAL_ERROR);
+
             log_warn(LD_PROTOCOL,
                      "Your application (using socks5 to port %d) gave Tor "
                      "a malformed hostname: %s. Rejecting the connection.",

src/test/test_socks.c

@@ -229,6 +229,42 @@ test_socks_5_supported_commands(void *ptr)
   tt_int_op(0,==, buf_datalen(buf));
   socks_request_clear(socks);
 
+  /* SOCKS 5 Should reject RESOLVE [F0] request for IPv4 address
+   * string if SafeSocks is enabled. */
+
+  ADD_DATA(buf, "\x05\x01\x00");
+  ADD_DATA(buf, "\x05\xF0\x00\x03\x07");
+  ADD_DATA(buf, "8.8.8.8");
+  ADD_DATA(buf, "\x01\x02");
+  tt_assert(fetch_from_buf_socks(buf,socks,get_options()->TestSocks,1)
+            == -1);
+
+  tt_int_op(5,==,socks->socks_version);
+  tt_int_op(10,==,socks->replylen);
+  tt_int_op(5,==,socks->reply[0]);
+  tt_int_op(SOCKS5_NOT_ALLOWED,==,socks->reply[1]);
+  tt_int_op(1,==,socks->reply[3]);
+
+  socks_request_clear(socks);
+
+  /* SOCKS 5 should reject RESOLVE [F0] reject for IPv6 address
+   * string if SafeSocks is enabled. */
+
+  ADD_DATA(buf, "\x05\x01\x00");
+  ADD_DATA(buf, "\x05\xF0\x00\x03\x27");
+  ADD_DATA(buf, "2001:0db8:85a3:0000:0000:8a2e:0370:7334");
+  ADD_DATA(buf, "\x01\x02");
+  tt_assert(fetch_from_buf_socks(buf,socks,get_options()->TestSocks,1)
+            == -1);
+
+  tt_int_op(5,==,socks->socks_version);
+  tt_int_op(10,==,socks->replylen);
+  tt_int_op(5,==,socks->reply[0]);
+  tt_int_op(SOCKS5_NOT_ALLOWED,==,socks->reply[1]);
+  tt_int_op(1,==,socks->reply[3]);
+
+  socks_request_clear(socks);
+
   /* SOCKS 5 Send RESOLVE_PTR [F1] for IP address 2.2.2.5 */
   ADD_DATA(buf, "\x05\x01\x00");
   ADD_DATA(buf, "\x05\xF1\x00\x01\x02\x02\x02\x05\x01\x03");

src/test/test_util.c

@@ -4790,6 +4790,52 @@ test_util_max_mem(void *arg)
   ;
 }
 
+static void
+test_util_hostname_validation(void *arg)
+{
+  (void)arg;
+
+  // Lets try valid hostnames first.
+  tt_assert(string_is_valid_hostname("torproject.org"));
+  tt_assert(string_is_valid_hostname("ocw.mit.edu"));
+  tt_assert(string_is_valid_hostname("i.4cdn.org"));
+  tt_assert(string_is_valid_hostname("stanford.edu"));
+  tt_assert(string_is_valid_hostname("multiple-words-with-hypens.jp"));
+
+  // Subdomain name cannot start with '-'.
+  tt_assert(!string_is_valid_hostname("-torproject.org"));
+  tt_assert(!string_is_valid_hostname("subdomain.-domain.org"));
+  tt_assert(!string_is_valid_hostname("-subdomain.domain.org"));
+
+  // Hostnames cannot contain non-alphanumeric characters.
+  tt_assert(!string_is_valid_hostname("%%domain.\\org."));
+  tt_assert(!string_is_valid_hostname("***x.net"));
+  tt_assert(!string_is_valid_hostname("___abc.org"));
+  tt_assert(!string_is_valid_hostname("\xff\xffxyz.org"));
+  tt_assert(!string_is_valid_hostname("word1 word2.net"));
+
+  // XXX: do we allow single-label DNS names?
+
+  done:
+  return;
+}
+
+static void
+test_util_ipv4_validation(void *arg)
+{
+  (void)arg;
+
+  tt_assert(string_is_valid_ipv4_address("192.168.0.1"));
+  tt_assert(string_is_valid_ipv4_address("8.8.8.8"));
+
+  tt_assert(!string_is_valid_ipv4_address("abcd"));
+  tt_assert(!string_is_valid_ipv4_address("300.300.300.300"));
+  tt_assert(!string_is_valid_ipv4_address("8.8."));
+
+  done:
+  return;
+}
+
 struct testcase_t util_tests[] = {
   UTIL_LEGACY(time),
   UTIL_TEST(parse_http_time, 0),
@@ -4863,6 +4909,8 @@ struct testcase_t util_tests[] = {
   { "socketpair_ersatz", test_util_socketpair, TT_FORK,
     &socketpair_setup, (void*)"1" },
   UTIL_TEST(max_mem, 0),
+  UTIL_TEST(hostname_validation, 0),
+  UTIL_TEST(ipv4_validation, 0),
   END_OF_TESTCASES
 };