Commit c4fe66e3 authored by Nick Mathewson's avatar Nick Mathewson 🦀
Browse files

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.
parent fcae26ad
Loading
Loading
Loading
Loading

changes/bug40190

0 → 100644
+4 −0
Original line number Diff line number Diff line
  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.
+6 −1
Original line number Diff line number Diff line
@@ -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;
      }