commit c5ad890904b68596d1081ce92307f57e67a5e3e4 Author: rl1987 rl1987@sdf.lonestar.org Date: Sun Sep 21 19:17:44 2014 +0300

Respond with 'Command not supported' SOCKS5 reply message upon reception of unsupported request. --- changes/bug12971 | 5 +++++ src/or/buffers.c | 20 ++++++++++++++++++++ src/test/test_socks.c | 18 ++++++++++++++---- 3 files changed, 39 insertions(+), 4 deletions(-)

diff --git a/changes/bug12971 b/changes/bug12971 new file mode 100644 index 0000000..e548bbf --- /dev/null +++ b/changes/bug12971 @@ -0,0 +1,5 @@ + o Bugfixes: + - Handle unsupported SOCKS5 requests properly by responding with + 'Command not supported' reply message before closing a TCP connection + to the user. Fixes bug 12971. + diff --git a/src/or/buffers.c b/src/or/buffers.c index 033f862..edb64ec 100644 --- a/src/or/buffers.c +++ b/src/or/buffers.c @@ -55,6 +55,9 @@ * forever. */

+static void send_socks5_error(socks_request_t *req, + socks5_reply_status_t reason); + static int parse_socks(const char *data, size_t datalen, socks_request_t *req, int log_sockstype, int safe_socks, ssize_t *drain_out, size_t *want_length_out); @@ -1831,6 +1834,21 @@ fetch_ext_or_command_from_evbuffer(struct evbuffer *buf, ext_or_cmd_t **out) } #endif

+/** Create a SOCKS5 reply message with <b>reason</b> in its REP field and + * have Tor send it as error response to <b>req</b>. + */ +static void +send_socks5_error(socks_request_t *req, + socks5_reply_status_t reason) +{ + req->replylen = 10; + memset(req->reply,0,10); + + req->reply[0] = 0x05; // VER field. + req->reply[1] = reason; // REP field. + req->reply[3] = 0x01; // ATYP field. +} + /** Implementation helper to implement fetch_from_*_socks. Instead of looking * at a buffer's contents, we look at the <b>datalen</b> bytes of data in * <b>data</b>. Instead of removing data from the buffer, we set @@ -1966,6 +1984,8 @@ parse_socks(const char *data, size_t datalen, socks_request_t *req, req->command != SOCKS_COMMAND_RESOLVE && req->command != SOCKS_COMMAND_RESOLVE_PTR) { /* not a connect or resolve or a resolve_ptr? we don't support it. */ + send_socks5_error(req,SOCKS5_COMMAND_NOT_SUPPORTED); + log_warn(LD_APP,"socks5: command %d not recognized. Rejecting.", req->command); return -1; diff --git a/src/test/test_socks.c b/src/test/test_socks.c index 20f58ca..2b8f824 100644 --- a/src/test/test_socks.c +++ b/src/test/test_socks.c @@ -143,23 +143,33 @@ test_socks_5_unsupported_commands(void *ptr) ADD_DATA(buf, "\x05\x02\x00\x01\x02\x02\x02\x01\x01\x01"); tt_int_op(fetch_from_buf_socks(buf, socks, get_options()->TestSocks, get_options()->SafeSocks),==, -1); - /* XXX: shouldn't tor reply 'command not supported' [07]? */ + + tt_int_op(5,==,socks->socks_version); + tt_int_op(10,==,socks->replylen); + tt_int_op(5,==,socks->reply[0]); + tt_int_op(SOCKS5_COMMAND_NOT_SUPPORTED,==,socks->reply[1]); + tt_int_op(1,==,socks->reply[3]);

buf_clear(buf); socks_request_clear(socks);

/* SOCKS 5 Send unsupported UDP_ASSOCIATE [03] command */ - ADD_DATA(buf, "\x05\x03\x00\x01\x02"); + ADD_DATA(buf, "\x05\x02\x00\x01"); tt_int_op(fetch_from_buf_socks(buf, socks, get_options()->TestSocks, get_options()->SafeSocks),==, 0); tt_int_op(5,==, socks->socks_version); tt_int_op(2,==, socks->replylen); tt_int_op(5,==, socks->reply[0]); - tt_int_op(2,==, socks->reply[1]); + tt_int_op(0,==, socks->reply[1]); ADD_DATA(buf, "\x05\x03\x00\x01\x02\x02\x02\x01\x01\x01"); tt_int_op(fetch_from_buf_socks(buf, socks, get_options()->TestSocks, get_options()->SafeSocks),==, -1); - /* XXX: shouldn't tor reply 'command not supported' [07]? */ + + tt_int_op(5,==,socks->socks_version); + tt_int_op(10,==,socks->replylen); + tt_int_op(5,==,socks->reply[0]); + tt_int_op(SOCKS5_COMMAND_NOT_SUPPORTED,==,socks->reply[1]); + tt_int_op(1,==,socks->reply[3]);

done: ;