From 0e453929d21b030832b0c48fceac0c5688657e15 Mon Sep 17 00:00:00 2001
From: rl1987 <rl1987@sdf.lonestar.org>
Date: Sun, 11 Feb 2018 15:22:41 +0100
Subject: [PATCH 01/17] Allow IPv6 address strings to be used as hostnames in
 SOCKS5 requests

---
 src/common/util.c     | 11 +++++++++++
 src/common/util.h     |  1 +
 src/or/proto_socks.c  |  4 ++--
 src/test/test_socks.c |  9 +++++----
 4 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/src/common/util.c b/src/common/util.c
index 90204befc..1818b4f19 100644
--- a/src/common/util.c
+++ b/src/common/util.c
@@ -1071,6 +1071,17 @@ string_is_valid_ipv6_address(const char *string)
   return (tor_inet_pton(AF_INET6,string,&addr) == 1);
 }
 
+/** Return true iff <b>string</b> is a valid destination address,
+ * i.e. either a DNS hostname or IPv4/IPv6 address string.
+ */
+int
+string_is_valid_dest(const char *string)
+{
+  return string_is_valid_ipv4_address(string) ||
+    string_is_valid_ipv6_address(string) ||
+    string_is_valid_hostname(string);
+}
+
 /** Return true iff <b>string</b> matches a pattern of DNS names
  * that we allow Tor clients to connect to.
  *
diff --git a/src/common/util.h b/src/common/util.h
index 2ee0ea28c..d6bda8036 100644
--- a/src/common/util.h
+++ b/src/common/util.h
@@ -233,6 +233,7 @@ 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_dest(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);
diff --git a/src/or/proto_socks.c b/src/or/proto_socks.c
index 91633d02a..8700fe126 100644
--- a/src/or/proto_socks.c
+++ b/src/or/proto_socks.c
@@ -393,7 +393,7 @@ parse_socks(const char *data, size_t datalen, socks_request_t *req,
           req->port = ntohs(get_uint16(data+5+len));
           *drain_out = 5+len+2;
 
-          if (!string_is_valid_hostname(req->address)) {
+          if (!string_is_valid_dest(req->address)) {
             socks_request_set_socks5_error(req, SOCKS5_GENERAL_ERROR);
 
             log_warn(LD_PROTOCOL,
@@ -518,7 +518,7 @@ parse_socks(const char *data, size_t datalen, socks_request_t *req,
       log_debug(LD_APP,"socks4: Everything is here. Success.");
       strlcpy(req->address, startaddr ? startaddr : tmpbuf,
               sizeof(req->address));
-      if (!string_is_valid_hostname(req->address)) {
+      if (!string_is_valid_dest(req->address)) {
         log_warn(LD_PROTOCOL,
                  "Your application (using socks4 to port %d) gave Tor "
                  "a malformed hostname: %s. Rejecting the connection.",
diff --git a/src/test/test_socks.c b/src/test/test_socks.c
index 9ae7530e2..70509e43e 100644
--- a/src/test/test_socks.c
+++ b/src/test/test_socks.c
@@ -347,17 +347,18 @@ test_socks_5_supported_commands(void *ptr)
 
   socks_request_clear(socks);
 
-  /* SOCKS 5 should NOT reject RESOLVE [F0] reject for IPv6 address
+  /* SOCKS 5 should NOT reject RESOLVE [F0] request 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, "\x05\xF0\x00\x03\x29");
+  ADD_DATA(buf, "[2001:0db8:85a3:0000:0000:8a2e:0370:7334]");
   ADD_DATA(buf, "\x01\x02");
   tt_int_op(fetch_from_buf_socks(buf, socks, get_options()->TestSocks, 1),
             OP_EQ, -1);
 
-  tt_str_op("2001:0db8:85a3:0000:0000:8a2e:0370:7334", OP_EQ, socks->address);
+  tt_str_op("[2001:0db8:85a3:0000:0000:8a2e:0370:7334]", OP_EQ,
+    socks->address);
   tt_int_op(258, OP_EQ, socks->port);
 
   tt_int_op(0, OP_EQ, buf_datalen(buf));

From 1af016e96e133718546b55c8b7fafd3345aaeeb8 Mon Sep 17 00:00:00 2001
From: rl1987 <rl1987@sdf.lonestar.org>
Date: Sun, 11 Feb 2018 16:39:23 +0100
Subject: [PATCH 02/17] Do not consider IP strings valid DNS names. Fixes
 #25055

---
 src/common/util.c    | 23 +++++++++++++++--------
 src/test/test_util.c |  5 +++++
 2 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/src/common/util.c b/src/common/util.c
index 1818b4f19..7c715fb3c 100644
--- a/src/common/util.c
+++ b/src/common/util.c
@@ -100,6 +100,8 @@
 #undef MALLOC_ZERO_WORKS
 #endif
 
+#include <ctype.h>
+
 /* =====
  * Memory management
  * ===== */
@@ -1110,16 +1112,21 @@ string_is_valid_hostname(const char *string)
       continue;
     }
 
-    do {
-      if ((*c >= 'a' && *c <= 'z') ||
-          (*c >= 'A' && *c <= 'Z') ||
-          (*c >= '0' && *c <= '9') ||
-          (*c == '-') || (*c == '_'))
+    if (c_sl_idx == c_sl_len - 1) {
+      do {
+        result = isalpha(*c);
         c++;
-      else
-        result = 0;
-    } while (result && *c);
+      } while (result && *c);
+    } else {
+      do {
+        result = (isalnum(*c) || (*c == '-') || (*c == '_'));
+        c++;
+      } while (result > 0 && *c);
+    }
 
+    if (result == 0) {
+      break;
+    }
   } SMARTLIST_FOREACH_END(c);
 
   SMARTLIST_FOREACH_BEGIN(components, char *, c) {
diff --git a/src/test/test_util.c b/src/test/test_util.c
index b67fad58e..2fa03e5bc 100644
--- a/src/test/test_util.c
+++ b/src/test/test_util.c
@@ -5584,6 +5584,11 @@ test_util_hostname_validation(void *arg)
   tt_assert(!string_is_valid_hostname("."));
   tt_assert(!string_is_valid_hostname(".."));
 
+  // IP address strings are not hostnames.
+  tt_assert(!string_is_valid_hostname("8.8.8.8"));
+  tt_assert(!string_is_valid_hostname("[2a00:1450:401b:800::200e]"));
+  tt_assert(!string_is_valid_hostname("2a00:1450:401b:800::200e"));
+
   done:
   return;
 }

From df529c60936ef290c917d09d51820680cd31cc8b Mon Sep 17 00:00:00 2001
From: rl1987 <rl1987@sdf.lonestar.org>
Date: Sun, 11 Feb 2018 18:16:51 +0100
Subject: [PATCH 03/17] Adding changes file

---
 changes/bugs_25036_25055 | 7 +++++++
 1 file changed, 7 insertions(+)
 create mode 100644 changes/bugs_25036_25055

diff --git a/changes/bugs_25036_25055 b/changes/bugs_25036_25055
new file mode 100644
index 000000000..daa46321c
--- /dev/null
+++ b/changes/bugs_25036_25055
@@ -0,0 +1,7 @@
+  o Minor bugfixes (networking):
+    - Tor will not reject IPv6 address strings from TorBrowser when they
+      are passed as hostnames in SOCKS5 requests. Fixes bug 25036,
+      bugfix on Tor 0.3.1.2.
+    - string_is_valid_hostname() will not consider IP strings to be valid
+      hostnames. Fixes bug 25055; bugfix on Tor 0.2.5.5.
+

From b0ba4aa7e98af030e0e1be19a58ab7a6f00fa423 Mon Sep 17 00:00:00 2001
From: rl1987 <rl1987@sdf.lonestar.org>
Date: Mon, 12 Feb 2018 19:52:47 +0100
Subject: [PATCH 04/17] Fix bracketed IPv6 string validation

---
 src/common/util.c     | 15 ++++++++++++++-
 src/test/test_socks.c |  2 +-
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/src/common/util.c b/src/common/util.c
index 7c715fb3c..ea0ec3dae 100644
--- a/src/common/util.c
+++ b/src/common/util.c
@@ -1079,9 +1079,22 @@ string_is_valid_ipv6_address(const char *string)
 int
 string_is_valid_dest(const char *string)
 {
-  return string_is_valid_ipv4_address(string) ||
+  char *tmp = NULL;
+  int retval;
+
+  tor_assert(string);
+  tor_assert(strlen(string) > 0);
+
+  if (string[0] == '[' && string[strlen(string) - 1] == ']')
+    string = tmp = tor_strndup(string + 1, strlen(string) - 2);
+
+  retval = string_is_valid_ipv4_address(string) ||
     string_is_valid_ipv6_address(string) ||
     string_is_valid_hostname(string);
+
+  tor_free(tmp);
+
+  return retval;
 }
 
 /** Return true iff <b>string</b> matches a pattern of DNS names
diff --git a/src/test/test_socks.c b/src/test/test_socks.c
index 70509e43e..3f9cc887b 100644
--- a/src/test/test_socks.c
+++ b/src/test/test_socks.c
@@ -355,7 +355,7 @@ test_socks_5_supported_commands(void *ptr)
   ADD_DATA(buf, "[2001:0db8:85a3:0000:0000:8a2e:0370:7334]");
   ADD_DATA(buf, "\x01\x02");
   tt_int_op(fetch_from_buf_socks(buf, socks, get_options()->TestSocks, 1),
-            OP_EQ, -1);
+            OP_EQ, 1);
 
   tt_str_op("[2001:0db8:85a3:0000:0000:8a2e:0370:7334]", OP_EQ,
     socks->address);

From 5986589b48de6addf99436df1feeea1362767acb Mon Sep 17 00:00:00 2001
From: rl1987 <rl1987@sdf.lonestar.org>
Date: Mon, 12 Feb 2018 21:08:17 +0100
Subject: [PATCH 05/17] Call strlen() once

---
 src/common/util.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/common/util.c b/src/common/util.c
index ea0ec3dae..d8891c6a5 100644
--- a/src/common/util.c
+++ b/src/common/util.c
@@ -1081,12 +1081,13 @@ string_is_valid_dest(const char *string)
 {
   char *tmp = NULL;
   int retval;
+  size_t len = strlen(string);
 
   tor_assert(string);
-  tor_assert(strlen(string) > 0);
+  tor_assert(len > 0);
 
-  if (string[0] == '[' && string[strlen(string) - 1] == ']')
-    string = tmp = tor_strndup(string + 1, strlen(string) - 2);
+  if (string[0] == '[' && string[len - 1] == ']')
+    string = tmp = tor_strndup(string + 1, len - 2);
 
   retval = string_is_valid_ipv4_address(string) ||
     string_is_valid_ipv6_address(string) ||

From 12afd8bfed96c57cd47b18644c2030673496a74f Mon Sep 17 00:00:00 2001
From: rl1987 <rl1987@sdf.lonestar.org>
Date: Mon, 12 Feb 2018 21:16:38 +0100
Subject: [PATCH 06/17] Also test bracket-less IPv6 string validation

---
 src/test/test_socks.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/src/test/test_socks.c b/src/test/test_socks.c
index 3f9cc887b..8da7191e8 100644
--- a/src/test/test_socks.c
+++ b/src/test/test_socks.c
@@ -365,6 +365,23 @@ test_socks_5_supported_commands(void *ptr)
 
   socks_request_clear(socks);
 
+  /* Also allow bracket-less form. */
+
+  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_int_op(fetch_from_buf_socks(buf, socks, get_options()->TestSocks, 1),
+            OP_EQ, 1);
+
+  tt_str_op("2001:0db8:85a3:0000:0000:8a2e:0370:7334", OP_EQ,
+    socks->address);
+  tt_int_op(258, OP_EQ, socks->port);
+
+  tt_int_op(0, OP_EQ, buf_datalen(buf));
+
+  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");

From 6335db9fce4275838c7de4bc10e522eb21a21ed8 Mon Sep 17 00:00:00 2001
From: rl1987 <rl1987@sdf.lonestar.org>
Date: Mon, 12 Feb 2018 21:26:57 +0100
Subject: [PATCH 07/17] Refrain from including <ctype.h>

---
 src/common/util.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/src/common/util.c b/src/common/util.c
index d8891c6a5..a7eaf5389 100644
--- a/src/common/util.c
+++ b/src/common/util.c
@@ -100,8 +100,6 @@
 #undef MALLOC_ZERO_WORKS
 #endif
 
-#include <ctype.h>
-
 /* =====
  * Memory management
  * ===== */
@@ -1128,12 +1126,12 @@ string_is_valid_hostname(const char *string)
 
     if (c_sl_idx == c_sl_len - 1) {
       do {
-        result = isalpha(*c);
+        result = TOR_ISALPHA(*c);
         c++;
       } while (result && *c);
     } else {
       do {
-        result = (isalnum(*c) || (*c == '-') || (*c == '_'));
+        result = (TOR_ISALNUM(*c) || (*c == '-') || (*c == '_'));
         c++;
       } while (result > 0 && *c);
     }

From db850fec3ac402084a9036c0ea7b4523f1120eb1 Mon Sep 17 00:00:00 2001
From: rl1987 <rl1987@sdf.lonestar.org>
Date: Mon, 12 Feb 2018 22:20:45 +0100
Subject: [PATCH 08/17] Test TLD validation

---
 src/test/test_util.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/src/test/test_util.c b/src/test/test_util.c
index 2fa03e5bc..db2ea1a34 100644
--- a/src/test/test_util.c
+++ b/src/test/test_util.c
@@ -5589,6 +5589,12 @@ test_util_hostname_validation(void *arg)
   tt_assert(!string_is_valid_hostname("[2a00:1450:401b:800::200e]"));
   tt_assert(!string_is_valid_hostname("2a00:1450:401b:800::200e"));
 
+  // Last label of a hostname is required to be alphabetic according to
+  // RFC 1123 Section 2.1.
+  tt_assert(!string_is_valid_hostname("lucky.13"));
+  tt_assert(!string_is_valid_hostname("luck.y13"));
+  tt_assert(!string_is_valid_hostname("luck.y13."));
+
   done:
   return;
 }

From 4413e52f9e2e3898f870df481aea93b1ea5fd836 Mon Sep 17 00:00:00 2001
From: rl1987 <rl1987@sdf.lonestar.org>
Date: Mon, 12 Feb 2018 22:21:10 +0100
Subject: [PATCH 09/17] Improve handling of trailing dot

---
 src/common/util.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/src/common/util.c b/src/common/util.c
index a7eaf5389..096188cfc 100644
--- a/src/common/util.c
+++ b/src/common/util.c
@@ -1106,24 +1106,33 @@ int
 string_is_valid_hostname(const char *string)
 {
   int result = 1;
+  int has_trailing_dot;
+  char *last_label;
   smartlist_t *components;
 
+  if (!string || strlen(string) == 0)
+    return 0;
+
   components = smartlist_new();
 
   smartlist_split_string(components,string,".",0,0);
 
+  /* Allow a single terminating '.' used rarely to indicate domains
+   * are FQDNs rather than relative. */
+  last_label = (char *)smartlist_get(components, smartlist_len(components) - 1);
+  has_trailing_dot = (last_label[0] == '\0');
+  if (has_trailing_dot) {
+    smartlist_pop_last(components);
+    tor_free(last_label);
+    last_label = NULL;
+  }
+
   SMARTLIST_FOREACH_BEGIN(components, char *, c) {
     if ((c[0] == '-') || (*c == '_')) {
       result = 0;
       break;
     }
 
-    /* Allow a single terminating '.' used rarely to indicate domains
-     * are FQDNs rather than relative. */
-    if ((c_sl_idx > 0) && (c_sl_idx + 1 == c_sl_len) && !*c) {
-      continue;
-    }
-
     if (c_sl_idx == c_sl_len - 1) {
       do {
         result = TOR_ISALPHA(*c);

From dbb7c8e6fd757db51226a47a2e14f4fd1aaf60c3 Mon Sep 17 00:00:00 2001
From: rl1987 <rl1987@sdf.lonestar.org>
Date: Sat, 17 Feb 2018 21:49:02 +0100
Subject: [PATCH 10/17] Validate hostnames with punycode TLDs correctly

---
 src/common/util.c    | 17 +++++++++++++----
 src/test/test_util.c |  4 ++++
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/src/common/util.c b/src/common/util.c
index 096188cfc..a55f7a3cd 100644
--- a/src/common/util.c
+++ b/src/common/util.c
@@ -1119,7 +1119,8 @@ string_is_valid_hostname(const char *string)
 
   /* Allow a single terminating '.' used rarely to indicate domains
    * are FQDNs rather than relative. */
-  last_label = (char *)smartlist_get(components, smartlist_len(components) - 1);
+  last_label = (char *)smartlist_get(components,
+                                     smartlist_len(components) - 1);
   has_trailing_dot = (last_label[0] == '\0');
   if (has_trailing_dot) {
     smartlist_pop_last(components);
@@ -1133,12 +1134,20 @@ string_is_valid_hostname(const char *string)
       break;
     }
 
-    if (c_sl_idx == c_sl_len - 1) {
+    if (c_sl_idx == c_sl_len - 1) { // TLD validation.
+      int is_punycode = (strlen(c) > 4 &&
+                         (c[0] == 'X' || c[0] == 'x') &&
+                         (c[1] == 'N' || c[1] == 'n') &&
+                          c[2] == '-' && c[3] == '-');
+
+      if (is_punycode)
+        c += 4;
+
       do {
-        result = TOR_ISALPHA(*c);
+        result = is_punycode ? TOR_ISALNUM(*c) : TOR_ISALPHA(*c);
         c++;
       } while (result && *c);
-    } else {
+    } else { // Regular hostname label validation.
       do {
         result = (TOR_ISALNUM(*c) || (*c == '-') || (*c == '_'));
         c++;
diff --git a/src/test/test_util.c b/src/test/test_util.c
index db2ea1a34..ef1f420fe 100644
--- a/src/test/test_util.c
+++ b/src/test/test_util.c
@@ -5595,6 +5595,10 @@ test_util_hostname_validation(void *arg)
   tt_assert(!string_is_valid_hostname("luck.y13"));
   tt_assert(!string_is_valid_hostname("luck.y13."));
 
+  // We allow punycode TLDs. For examples, see
+  // http://data.iana.org/TLD/tlds-alpha-by-domain.txt
+  tt_assert(string_is_valid_hostname("example.xn--l1acc"));
+
   done:
   return;
 }

From ee1fca727cd739ba94c215a4a45a416bfcc8956e Mon Sep 17 00:00:00 2001
From: rl1987 <rl1987@sdf.lonestar.org>
Date: Mon, 19 Feb 2018 21:08:51 +0100
Subject: [PATCH 11/17] Simplify hostname validation code

---
 src/common/util.c | 26 +++++++-------------------
 1 file changed, 7 insertions(+), 19 deletions(-)

diff --git a/src/common/util.c b/src/common/util.c
index a55f7a3cd..1402462fb 100644
--- a/src/common/util.c
+++ b/src/common/util.c
@@ -1113,6 +1113,9 @@ string_is_valid_hostname(const char *string)
   if (!string || strlen(string) == 0)
     return 0;
 
+  if (string_is_valid_ipv4_address(string))
+    return 0;
+
   components = smartlist_new();
 
   smartlist_split_string(components,string,".",0,0);
@@ -1134,25 +1137,10 @@ string_is_valid_hostname(const char *string)
       break;
     }
 
-    if (c_sl_idx == c_sl_len - 1) { // TLD validation.
-      int is_punycode = (strlen(c) > 4 &&
-                         (c[0] == 'X' || c[0] == 'x') &&
-                         (c[1] == 'N' || c[1] == 'n') &&
-                          c[2] == '-' && c[3] == '-');
-
-      if (is_punycode)
-        c += 4;
-
-      do {
-        result = is_punycode ? TOR_ISALNUM(*c) : TOR_ISALPHA(*c);
-        c++;
-      } while (result && *c);
-    } else { // Regular hostname label validation.
-      do {
-        result = (TOR_ISALNUM(*c) || (*c == '-') || (*c == '_'));
-        c++;
-      } while (result > 0 && *c);
-    }
+    do {
+      result = (TOR_ISALNUM(*c) || (*c == '-') || (*c == '_'));
+      c++;
+    } while (result > 0 && *c);
 
     if (result == 0) {
       break;

From d891010fdd4562e29a5a468232cd7b30430d7570 Mon Sep 17 00:00:00 2001
From: rl1987 <rl1987@sdf.lonestar.org>
Date: Tue, 20 Feb 2018 19:52:48 +0100
Subject: [PATCH 12/17] Allow alphanumeric TLDs in test for now

---
 src/test/test_util.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/src/test/test_util.c b/src/test/test_util.c
index ef1f420fe..ee9b16494 100644
--- a/src/test/test_util.c
+++ b/src/test/test_util.c
@@ -5589,11 +5589,10 @@ test_util_hostname_validation(void *arg)
   tt_assert(!string_is_valid_hostname("[2a00:1450:401b:800::200e]"));
   tt_assert(!string_is_valid_hostname("2a00:1450:401b:800::200e"));
 
-  // Last label of a hostname is required to be alphabetic according to
-  // RFC 1123 Section 2.1.
-  tt_assert(!string_is_valid_hostname("lucky.13"));
-  tt_assert(!string_is_valid_hostname("luck.y13"));
-  tt_assert(!string_is_valid_hostname("luck.y13."));
+  // We allow alphanumeric TLDs. For discussion, see ticket #25055.
+  tt_assert(string_is_valid_hostname("lucky.13"));
+  tt_assert(string_is_valid_hostname("luck.y13"));
+  tt_assert(string_is_valid_hostname("luck.y13."));
 
   // We allow punycode TLDs. For examples, see
   // http://data.iana.org/TLD/tlds-alpha-by-domain.txt

From 6b6d003f43cbbf01b40cedb0cc12ada2e81461f9 Mon Sep 17 00:00:00 2001
From: rl1987 <rl1987@sdf.lonestar.org>
Date: Wed, 21 Feb 2018 20:23:21 +0100
Subject: [PATCH 13/17] Don't explode on NULL or empty string

---
 src/common/util.c    |  9 +++++++--
 src/test/test_util.c | 13 +++++++++++++
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/src/common/util.c b/src/common/util.c
index 1402462fb..53e117f24 100644
--- a/src/common/util.c
+++ b/src/common/util.c
@@ -1081,8 +1081,13 @@ string_is_valid_dest(const char *string)
   int retval;
   size_t len = strlen(string);
 
-  tor_assert(string);
-  tor_assert(len > 0);
+  if (string == NULL)
+    return 0;
+
+  len = strlen(string);
+
+  if (len == 0)
+    return 0;
 
   if (string[0] == '[' && string[len - 1] == ']')
     string = tmp = tor_strndup(string + 1, len - 2);
diff --git a/src/test/test_util.c b/src/test/test_util.c
index ee9b16494..c734426a5 100644
--- a/src/test/test_util.c
+++ b/src/test/test_util.c
@@ -5541,6 +5541,18 @@ test_util_max_mem(void *arg)
   ;
 }
 
+static void
+test_util_dest_validation_edgecase(void *arg)
+{
+  (void)arg;
+
+  tt_assert(!string_is_valid_dest(NULL));
+  tt_assert(!string_is_valid_dest(""));
+
+  done:
+  return;
+}
+
 static void
 test_util_hostname_validation(void *arg)
 {
@@ -6222,6 +6234,7 @@ struct testcase_t util_tests[] = {
     &passthrough_setup, (void*)"1" },
   UTIL_TEST(max_mem, 0),
   UTIL_TEST(hostname_validation, 0),
+  UTIL_TEST(dest_validation_edgecase, 0),
   UTIL_TEST(ipv4_validation, 0),
   UTIL_TEST(writepid, 0),
   UTIL_TEST(get_avail_disk_space, 0),

From a28e350cff1572a1e5a0c5df93f6e6005904689a Mon Sep 17 00:00:00 2001
From: rl1987 <rl1987@sdf.lonestar.org>
Date: Wed, 21 Feb 2018 20:25:24 +0100
Subject: [PATCH 14/17] Tweak loop condition

---
 src/common/util.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/common/util.c b/src/common/util.c
index 53e117f24..497b60be1 100644
--- a/src/common/util.c
+++ b/src/common/util.c
@@ -1145,7 +1145,7 @@ string_is_valid_hostname(const char *string)
     do {
       result = (TOR_ISALNUM(*c) || (*c == '-') || (*c == '_'));
       c++;
-    } while (result > 0 && *c);
+    } while (result && *c);
 
     if (result == 0) {
       break;

From 09351c34e9bea4d29fb6f4ac8d40e3bee49e12fc Mon Sep 17 00:00:00 2001
From: rl1987 <rl1987@sdf.lonestar.org>
Date: Thu, 22 Feb 2018 19:52:40 +0100
Subject: [PATCH 15/17] Don't strlen before checking for NULL

---
 src/common/util.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/common/util.c b/src/common/util.c
index 497b60be1..5ae4d0408 100644
--- a/src/common/util.c
+++ b/src/common/util.c
@@ -1079,7 +1079,7 @@ string_is_valid_dest(const char *string)
 {
   char *tmp = NULL;
   int retval;
-  size_t len = strlen(string);
+  size_t len;
 
   if (string == NULL)
     return 0;

From b504c854d34a938943f68c3036840f10a84fcea4 Mon Sep 17 00:00:00 2001
From: Nick Mathewson <nickm@torproject.org>
Date: Wed, 28 Mar 2018 07:42:27 -0400
Subject: [PATCH 16/17] Rename string_is_valid_hostname ->
 string_is_valid_nonrfc_hostname

Per discussion on 25055.
---
 src/common/util.c    |  4 ++--
 src/common/util.h    |  2 +-
 src/test/test_util.c | 55 ++++++++++++++++++++++----------------------
 3 files changed, 31 insertions(+), 30 deletions(-)

diff --git a/src/common/util.c b/src/common/util.c
index 5ae4d0408..90aaf0ebe 100644
--- a/src/common/util.c
+++ b/src/common/util.c
@@ -1094,7 +1094,7 @@ string_is_valid_dest(const char *string)
 
   retval = string_is_valid_ipv4_address(string) ||
     string_is_valid_ipv6_address(string) ||
-    string_is_valid_hostname(string);
+    string_is_valid_nonrfc_hostname(string);
 
   tor_free(tmp);
 
@@ -1108,7 +1108,7 @@ string_is_valid_dest(const char *string)
  * with misconfigured zones that have been encountered in the wild.
  */
 int
-string_is_valid_hostname(const char *string)
+string_is_valid_nonrfc_hostname(const char *string)
 {
   int result = 1;
   int has_trailing_dot;
diff --git a/src/common/util.h b/src/common/util.h
index d6bda8036..938078912 100644
--- a/src/common/util.h
+++ b/src/common/util.h
@@ -234,7 +234,7 @@ const char *find_str_at_start_of_line(const char *haystack,
 int string_is_C_identifier(const char *string);
 int string_is_key_value(int severity, const char *string);
 int string_is_valid_dest(const char *string);
-int string_is_valid_hostname(const char *string);
+int string_is_valid_nonrfc_hostname(const char *string);
 int string_is_valid_ipv4_address(const char *string);
 int string_is_valid_ipv6_address(const char *string);
 
diff --git a/src/test/test_util.c b/src/test/test_util.c
index c734426a5..036f739b8 100644
--- a/src/test/test_util.c
+++ b/src/test/test_util.c
@@ -5559,56 +5559,57 @@ 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"));
+  tt_assert(string_is_valid_nonrfc_hostname("torproject.org"));
+  tt_assert(string_is_valid_nonrfc_hostname("ocw.mit.edu"));
+  tt_assert(string_is_valid_nonrfc_hostname("i.4cdn.org"));
+  tt_assert(string_is_valid_nonrfc_hostname("stanford.edu"));
+  tt_assert(string_is_valid_nonrfc_hostname("multiple-words-with-hypens.jp"));
 
   // Subdomain name cannot start with '-' or '_'.
-  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"));
-  tt_assert(!string_is_valid_hostname("___abc.org"));
+  tt_assert(!string_is_valid_nonrfc_hostname("-torproject.org"));
+  tt_assert(!string_is_valid_nonrfc_hostname("subdomain.-domain.org"));
+  tt_assert(!string_is_valid_nonrfc_hostname("-subdomain.domain.org"));
+  tt_assert(!string_is_valid_nonrfc_hostname("___abc.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("\xff\xffxyz.org"));
-  tt_assert(!string_is_valid_hostname("word1 word2.net"));
+  tt_assert(!string_is_valid_nonrfc_hostname("%%domain.\\org."));
+  tt_assert(!string_is_valid_nonrfc_hostname("***x.net"));
+  tt_assert(!string_is_valid_nonrfc_hostname("\xff\xffxyz.org"));
+  tt_assert(!string_is_valid_nonrfc_hostname("word1 word2.net"));
 
   // Test workaround for nytimes.com stupidity, technically invalid,
   // but we allow it since they are big, even though they are failing to
   // comply with a ~30 year old standard.
-  tt_assert(string_is_valid_hostname("core3_euw1.fabrik.nytimes.com"));
+  tt_assert(string_is_valid_nonrfc_hostname("core3_euw1.fabrik.nytimes.com"));
 
   // Firefox passes FQDNs with trailing '.'s  directly to the SOCKS proxy,
   // which is redundant since the spec states DOMAINNAME addresses are fully
   // qualified.  While unusual, this should be tollerated.
-  tt_assert(string_is_valid_hostname("core9_euw1.fabrik.nytimes.com."));
-  tt_assert(!string_is_valid_hostname("..washingtonpost.is.better.com"));
-  tt_assert(!string_is_valid_hostname("so.is..ft.com"));
-  tt_assert(!string_is_valid_hostname("..."));
+  tt_assert(string_is_valid_nonrfc_hostname("core9_euw1.fabrik.nytimes.com."));
+  tt_assert(!string_is_valid_nonrfc_hostname(
+                                         "..washingtonpost.is.better.com"));
+  tt_assert(!string_is_valid_nonrfc_hostname("so.is..ft.com"));
+  tt_assert(!string_is_valid_nonrfc_hostname("..."));
 
   // XXX: do we allow single-label DNS names?
   // We shouldn't for SOCKS (spec says "contains a fully-qualified domain name"
   // but only test pathologically malformed traling '.' cases for now.
-  tt_assert(!string_is_valid_hostname("."));
-  tt_assert(!string_is_valid_hostname(".."));
+  tt_assert(!string_is_valid_nonrfc_hostname("."));
+  tt_assert(!string_is_valid_nonrfc_hostname(".."));
 
   // IP address strings are not hostnames.
-  tt_assert(!string_is_valid_hostname("8.8.8.8"));
-  tt_assert(!string_is_valid_hostname("[2a00:1450:401b:800::200e]"));
-  tt_assert(!string_is_valid_hostname("2a00:1450:401b:800::200e"));
+  tt_assert(!string_is_valid_nonrfc_hostname("8.8.8.8"));
+  tt_assert(!string_is_valid_nonrfc_hostname("[2a00:1450:401b:800::200e]"));
+  tt_assert(!string_is_valid_nonrfc_hostname("2a00:1450:401b:800::200e"));
 
   // We allow alphanumeric TLDs. For discussion, see ticket #25055.
-  tt_assert(string_is_valid_hostname("lucky.13"));
-  tt_assert(string_is_valid_hostname("luck.y13"));
-  tt_assert(string_is_valid_hostname("luck.y13."));
+  tt_assert(string_is_valid_nonrfc_hostname("lucky.13"));
+  tt_assert(string_is_valid_nonrfc_hostname("luck.y13"));
+  tt_assert(string_is_valid_nonrfc_hostname("luck.y13."));
 
   // We allow punycode TLDs. For examples, see
   // http://data.iana.org/TLD/tlds-alpha-by-domain.txt
-  tt_assert(string_is_valid_hostname("example.xn--l1acc"));
+  tt_assert(string_is_valid_nonrfc_hostname("example.xn--l1acc"));
 
   done:
   return;

From d4bf1f6c8eb08c39def69c839515afe475bf0a6b Mon Sep 17 00:00:00 2001
From: Nick Mathewson <nickm@torproject.org>
Date: Wed, 28 Mar 2018 07:48:18 -0400
Subject: [PATCH 17/17] Add a paranoia check in
 string_is_valid_nonrfc_hostname()

The earlier checks in this function should ensure that components is
always nonempty.  But in case somebody messes with them in the
future, let's add an extra check to make sure we aren't crashing.
---
 src/common/util.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/common/util.c b/src/common/util.c
index 90aaf0ebe..a68fd30d0 100644
--- a/src/common/util.c
+++ b/src/common/util.c
@@ -1125,6 +1125,9 @@ string_is_valid_nonrfc_hostname(const char *string)
 
   smartlist_split_string(components,string,".",0,0);
 
+  if (BUG(smartlist_len(components) == 0))
+    return 0; // LCOV_EXCL_LINE should be impossible given the earlier checks.
+
   /* Allow a single terminating '.' used rarely to indicate domains
    * are FQDNs rather than relative. */
   last_label = (char *)smartlist_get(components,