[tor-commits] [tor/master] Send back SOCKS5 errors for all of the address related failures.
nickm at torproject.org
nickm at torproject.org
Thu Oct 16 13:16:54 UTC 2014
commit c8132aab9291527db2ba0524fbd84a3041bcd6fd
Author: Yawning Angel <yawning at schwanenlied.me>
Date: Wed Oct 1 14:16:59 2014 +0000
Send back SOCKS5 errors for all of the address related failures.
Cases that now send errors:
* Malformed IP address (SOCKS5_GENERAL_ERROR)
* CONNECT/RESOLVE request with IP, when SafeSocks is set
(SOCKS5_NOT_ALLOWED)
* RESOLVE_PTR request with FQDN (SOCKS5_ADDRESS_TYPE_NOT_SUPPORTED)
* Malformed FQDN (SOCKS5_GENERAL_ERROR)
* Unknown address type (SOCKS5_ADDRESS_TYPE_NOT_SUPPORTED)
Fixes bug 13314.
---
changes/bug13314 | 4 +++
src/or/buffers.c | 11 +++++++-
src/test/test_socks.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 86 insertions(+), 1 deletion(-)
diff --git a/changes/bug13314 b/changes/bug13314
new file mode 100644
index 0000000..e9017fa
--- /dev/null
+++ b/changes/bug13314
@@ -0,0 +1,4 @@
+ o Bugfixes:
+ - Handle malformed SOCKS5 requests properly by responding with an
+ appropriate error message before closing a TCP connection to the
+ user. Fixes bug 13314.
diff --git a/src/or/buffers.c b/src/or/buffers.c
index d174f81..9f02824 100644
--- a/src/or/buffers.c
+++ b/src/or/buffers.c
@@ -2009,6 +2009,7 @@ parse_socks(const char *data, size_t datalen, socks_request_t *req,
tor_addr_to_str(tmpbuf, &destaddr, sizeof(tmpbuf), 1);
if (strlen(tmpbuf)+1 > MAX_SOCKS_ADDR_LEN) {
+ socks_request_set_socks5_error(req, SOCKS5_GENERAL_ERROR);
log_warn(LD_APP,
"socks5 IP takes %d bytes, which doesn't fit in %d. "
"Rejecting.",
@@ -2021,14 +2022,18 @@ parse_socks(const char *data, size_t datalen, socks_request_t *req,
if (req->command != SOCKS_COMMAND_RESOLVE_PTR &&
!addressmap_have_mapping(req->address,0)) {
log_unsafe_socks_warning(5, req->address, req->port, safe_socks);
- if (safe_socks)
+ if (safe_socks) {
+ socks_request_set_socks5_error(req, SOCKS5_NOT_ALLOWED);
return -1;
+ }
}
return 1;
}
case 3: /* fqdn */
log_debug(LD_APP,"socks5: fqdn address type");
if (req->command == SOCKS_COMMAND_RESOLVE_PTR) {
+ socks_request_set_socks5_error(req,
+ SOCKS5_ADDRESS_TYPE_NOT_SUPPORTED);
log_warn(LD_APP, "socks5 received RESOLVE_PTR command with "
"hostname type. Rejecting.");
return -1;
@@ -2039,6 +2044,7 @@ parse_socks(const char *data, size_t datalen, socks_request_t *req,
return 0; /* not yet */
}
if (len+1 > MAX_SOCKS_ADDR_LEN) {
+ socks_request_set_socks5_error(req, SOCKS5_GENERAL_ERROR);
log_warn(LD_APP,
"socks5 hostname is %d bytes, which doesn't fit in "
"%d. Rejecting.", len+1,MAX_SOCKS_ADDR_LEN);
@@ -2049,6 +2055,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 (!tor_strisprint(req->address) || strchr(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.",
@@ -2062,6 +2069,8 @@ parse_socks(const char *data, size_t datalen, socks_request_t *req,
"necessary. This is good.", req->port);
return 1;
default: /* unsupported */
+ socks_request_set_socks5_error(req,
+ SOCKS5_ADDRESS_TYPE_NOT_SUPPORTED);
log_warn(LD_APP,"socks5: unsupported address type %d. Rejecting.",
(int) *(data+3));
return -1;
diff --git a/src/test/test_socks.c b/src/test/test_socks.c
index 2b8f824..9aaa16e 100644
--- a/src/test/test_socks.c
+++ b/src/test/test_socks.c
@@ -384,6 +384,77 @@ test_socks_5_auth_before_negotiation(void *ptr)
;
}
+/** Perform malformed SOCKS 5 commands */
+static void
+test_socks_5_malformed_commands(void *ptr)
+{
+ SOCKS_TEST_INIT();
+
+ /* XXX: Stringified address length > MAX_SOCKS_ADDR_LEN will never happen */
+
+ /* SOCKS 5 Send CONNECT [01] to IP address 2.2.2.2:4369, with SafeSocks set */
+ ADD_DATA(buf, "\x05\x01\x00");
+ ADD_DATA(buf, "\x05\x01\x00\x01\x02\x02\x02\x02\x11\x11");
+ tt_int_op(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]);
+
+ buf_clear(buf);
+ socks_request_clear(socks);
+
+ /* SOCKS 5 Send RESOLVE_PTR [F1] for FQDN torproject.org */
+ ADD_DATA(buf, "\x05\x01\x00");
+ ADD_DATA(buf, "\x05\xF1\x00\x03\x0Etorproject.org\x11\x11");
+ tt_int_op(fetch_from_buf_socks(buf, socks, get_options()->TestSocks,
+ get_options()->SafeSocks),==, -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_ADDRESS_TYPE_NOT_SUPPORTED,==,socks->reply[1]);
+ tt_int_op(1,==,socks->reply[3]);
+
+ buf_clear(buf);
+ socks_request_clear(socks);
+
+ /* XXX: len + 1 > MAX_SOCKS_ADDR_LEN (FQDN request) will never happen */
+
+ /* SOCKS 5 Send CONNECT [01] to FQDN """"".com */
+ ADD_DATA(buf, "\x05\x01\x00");
+ ADD_DATA(buf, "\x05\x01\x00\x03\x09\"\"\"\"\".com\x11\x11");
+ tt_int_op(fetch_from_buf_socks(buf, socks, get_options()->TestSocks,
+ get_options()->SafeSocks),==, -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_GENERAL_ERROR,==,socks->reply[1]);
+ tt_int_op(1,==,socks->reply[3]);
+
+ buf_clear(buf);
+ socks_request_clear(socks);
+
+ /* SOCKS 5 Send CONNECT [01] to address type 0x23 */
+ ADD_DATA(buf, "\x05\x01\x00");
+ ADD_DATA(buf, "\x05\x01\x00\x23\x02\x02\x02\x02\x11\x11");
+ tt_int_op(fetch_from_buf_socks(buf, socks, get_options()->TestSocks,
+ get_options()->SafeSocks),==, -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_ADDRESS_TYPE_NOT_SUPPORTED,==,socks->reply[1]);
+ tt_int_op(1,==,socks->reply[3]);
+
+ done:
+ ;
+}
+
#define SOCKSENT(name) \
{ #name, test_socks_##name, TT_FORK, &socks_setup, NULL }
@@ -397,6 +468,7 @@ struct testcase_t socks_tests[] = {
SOCKSENT(5_auth_before_negotiation),
SOCKSENT(5_authenticate),
SOCKSENT(5_authenticate_with_data),
+ SOCKSENT(5_malformed_commands),
END_OF_TESTCASES
};
More information about the tor-commits
mailing list