[tor-commits] [tor/master] Don't reject SOCKS5 requests that contain IP strings

[nickm at torproject.org]

commit 7f05f896630e857ad2803e80b48924f026f66eb7
Author: rl1987 <rl1987 at sdf.lonestar.org>
Date:   Sun Jun 4 13:14:55 2017 +0200

    Don't reject SOCKS5 requests that contain IP strings
---
 changes/bug22461      |  7 ++++---
 src/or/buffers.c      |  8 +-------
 src/test/test_socks.c | 26 ++++++++++++--------------
 3 files changed, 17 insertions(+), 24 deletions(-)

diff --git a/changes/bug22461 b/changes/bug22461
index 2fb6a02..343a2b4 100644
--- a/changes/bug22461
+++ b/changes/bug22461
@@ -1,4 +1,5 @@
   o Minor bugfixes:
-    - Refrain from needlessly warning Tor controller about passing
-      IP addresses as FQDNs through SOCKS5 interface. Fixes bug
-      22461, bugfix on Tor 0.2.6.2-alpha.
+    - Refrain from needlessly rejecting SOCKS5 requests that contain
+      IP address strings when SafeSocks in enabled as this prevents
+      user from connecting to IP address they know without relying on
+      DNS for resolving.  Fixes bug 22461, bugfix on Tor 0.2.6.2-alpha.
diff --git a/src/or/buffers.c b/src/or/buffers.c
index 1df4be1..399b591 100644
--- a/src/or/buffers.c
+++ b/src/or/buffers.c
@@ -1684,13 +1684,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_ipv4_address(req->address) ||
-              string_is_valid_ipv6_address(req->address)) {
-            if (safe_socks) {
-              socks_request_set_socks5_error(req, SOCKS5_NOT_ALLOWED);
-              return -1;
-            }
-          } else if (!string_is_valid_hostname(req->address)) {
+          if (!string_is_valid_hostname(req->address)) {
             socks_request_set_socks5_error(req, SOCKS5_GENERAL_ERROR);
 
             log_warn(LD_PROTOCOL,
diff --git a/src/test/test_socks.c b/src/test/test_socks.c
index bb1be11..ab2393c 100644
--- a/src/test/test_socks.c
+++ b/src/test/test_socks.c
@@ -229,25 +229,24 @@ test_socks_5_supported_commands(void *ptr)
   tt_int_op(0,OP_EQ, buf_datalen(buf));
   socks_request_clear(socks);
 
-  /* SOCKS 5 Should reject RESOLVE [F0] request for IPv4 address
+  /* SOCKS 5 Should NOT 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");
+  ADD_DATA(buf, "\x11\x11");
   tt_assert(fetch_from_buf_socks(buf,socks,get_options()->TestSocks,1)
-            == -1);
+            == 1);
 
-  tt_int_op(5,OP_EQ,socks->socks_version);
-  tt_int_op(10,OP_EQ,socks->replylen);
-  tt_int_op(5,OP_EQ,socks->reply[0]);
-  tt_int_op(SOCKS5_NOT_ALLOWED,OP_EQ,socks->reply[1]);
-  tt_int_op(1,OP_EQ,socks->reply[3]);
+  tt_str_op("8.8.8.8", OP_EQ, socks->address);
+  tt_int_op(4369, OP_EQ, socks->port);
+  
+  tt_int_op(0, OP_EQ, buf_datalen(buf));
 
   socks_request_clear(socks);
 
-  /* SOCKS 5 should reject RESOLVE [F0] reject for IPv6 address
+  /* SOCKS 5 should NOT reject RESOLVE [F0] reject for IPv6 address
    * string if SafeSocks is enabled. */
 
   ADD_DATA(buf, "\x05\x01\x00");
@@ -257,11 +256,10 @@ test_socks_5_supported_commands(void *ptr)
   tt_assert(fetch_from_buf_socks(buf,socks,get_options()->TestSocks,1)
             == -1);
 
-  tt_int_op(5,OP_EQ,socks->socks_version);
-  tt_int_op(10,OP_EQ,socks->replylen);
-  tt_int_op(5,OP_EQ,socks->reply[0]);
-  tt_int_op(SOCKS5_NOT_ALLOWED,OP_EQ,socks->reply[1]);
-  tt_int_op(1,OP_EQ,socks->reply[3]);
+  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);