[tor-commits] [tor/master] Allow a single trailing `.` when validating FQDNs from SOCKS.

[nickm at torproject.org]

commit da6aa7bfa5014b980a93b38024d16b32720dc67a
Author: Yawning Angel <yawning at schwanenlied.me>
Date:   Mon Jul 27 12:58:40 2015 +0000

    Allow a single trailing `.` when validating FQDNs from SOCKS.
    
    URI syntax (and DNS syntax) allows for a single trailing `.` to
    explicitly distinguish between a relative and absolute
    (fully-qualified) domain name. While this is redundant in that RFC 1928
    DOMAINNAME addresses are *always* fully-qualified, certain clients
    blindly pass the trailing `.` along in the request.
    
    Fixes bug 16674; bugfix on 0.2.6.2-alpha.
---
 changes/bug16674     |    5 +++++
 src/common/util.c    |    6 ++++++
 src/test/test_util.c |   12 ++++++++++++
 3 files changed, 23 insertions(+)

diff --git a/changes/bug16674 b/changes/bug16674
new file mode 100644
index 0000000..de55523
--- /dev/null
+++ b/changes/bug16674
@@ -0,0 +1,5 @@
+  o Minor features (client):
+    - Relax the validation done to hostnames in SOCKS5 requests, and allow
+      a single trailing '.' to cope with clients that pass FQDNs using that
+      syntax to explicitly indicate that the domain name is
+      fully-qualified. Fixes bug 16674; bugfix on 0.2.6.2-alpha.
diff --git a/src/common/util.c b/src/common/util.c
index 618e6a1..1aac4fc 100644
--- a/src/common/util.c
+++ b/src/common/util.c
@@ -1056,6 +1056,12 @@ string_is_valid_hostname(const char *string)
       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;
+    }
+
     do {
       if ((*c >= 'a' && *c <= 'z') ||
           (*c >= 'A' && *c <= 'Z') ||
diff --git a/src/test/test_util.c b/src/test/test_util.c
index 0f64c26..2bffb17 100644
--- a/src/test/test_util.c
+++ b/src/test/test_util.c
@@ -4285,7 +4285,19 @@ test_util_hostname_validation(void *arg)
   // comply with a ~30 year old standard.
   tt_assert(string_is_valid_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("..."));
+
   // 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(".."));
 
   done:
   return;