Socks5: handle truncated client requests correctly

Previously, our code would send back an error if the socks5 request
parser said anything but DONE.  But there are other non-error cases,
like TRUNCATED: we shouldn't send back errors for them.

This patch lowers the responsibility for setting the error message
into the parsing code, since the actual type of the error message
will depend on what problem was encountered.

Fixes bug 40190; bugfix on 0.3.5.1-alpha.

2 files changed, 10 insertions, 1 deletions

diff --git a/changes/bug40190 b/changes/bug40190
new file mode 100644
index 0000000000..0f3d6941dc
--- /dev/null
+++ b/changes/bug40190
@@ -0,0 +1,4 @@
+  o Minor bugfixes (SOCKS5):
+    - Handle partial socks5 messages correctly.  Previously, our code would
+      send an incorrect error message if it got a socks5 request that wasn't
+      complete. Fixes bug 40190; bugfix on 0.3.5.1-alpha.
diff --git a/src/core/proto/proto_socks.c b/src/core/proto/proto_socks.c
index c7bf13b9f4..5a7d7ac9be 100644
--- a/src/core/proto/proto_socks.c
+++ b/src/core/proto/proto_socks.c
@@ -545,6 +545,7 @@ parse_socks5_client_request(const uint8_t *raw_data, socks_request_t *req,
   if (parsed == -1) {
     log_warn(LD_APP, "socks5: parsing failed - invalid client request");
     res = SOCKS_RESULT_INVALID;
+    socks_request_set_socks5_error(req, SOCKS5_GENERAL_ERROR);
     goto end;
   } else if (parsed == -2) {
     res = SOCKS_RESULT_TRUNCATED;
@@ -556,6 +557,7 @@ parse_socks5_client_request(const uint8_t *raw_data, socks_request_t *req,
 
   if (socks5_client_request_get_version(trunnel_req) != 5) {
     res = SOCKS_RESULT_INVALID;
+    socks_request_set_socks5_error(req, SOCKS5_GENERAL_ERROR);
     goto end;
   }
 
@@ -590,6 +592,7 @@ parse_socks5_client_request(const uint8_t *raw_data, socks_request_t *req,
       tor_addr_to_str(req->address, &destaddr, sizeof(req->address), 1);
     } break;
     default: {
+      socks_request_set_socks5_error(req, SOCKS5_ADDRESS_TYPE_NOT_SUPPORTED);
       res = -1;
     } break;
   }
@@ -770,8 +773,10 @@ handle_socks_message(const uint8_t *raw_data, size_t datalen,
     } else {
       res = parse_socks5_client_request(raw_data, req,
                                         datalen, drain_out);
-      if (res != SOCKS_RESULT_DONE) {
+      if (BUG(res == SOCKS_RESULT_INVALID && req->replylen == 0)) {
         socks_request_set_socks5_error(req, SOCKS5_GENERAL_ERROR);
+      }
+      if (res != SOCKS_RESULT_DONE) {
         goto end;
       }