[tor-commits] [tor/master] Respond with 'Command not supported' SOCKS5 reply message upon reception of unsupported request.

[nickm at torproject.org]

commit c5ad890904b68596d1081ce92307f57e67a5e3e4
Author: rl1987 <rl1987 at 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:
   ;