commit 7f05f896630e857ad2803e80b48924f026f66eb7 Author: rl1987 <rl1987@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);