socks4a: simplify parsing to use trunnel hostname output.

The previous comment about not using trunnel is wrong; we can still use the
hostname field in trunnel, but we just have to check its length.

Fixes bug 41190 (which I think is a false positive).
Bugfix on 0.3.5.1-alpha.

diff --git a/changes/ticket_41190 b/changes/ticket_41190
new file mode 100644 (file)
index 0000000..223b0fb
--- /dev/null
+++ b/changes/ticket_41190
@@ -0,0 +1,4 @@
+  o Code simplification and refactoring:
+    - Simplify SOCKS4a parsing to avoid the (false) appearance of
+      integer underflows, and to make the logic more obvious.
+      Fixes bug 41190; bugfix on 0.3.5.1-alpha.

diff --git a/src/core/proto/proto_socks.c b/src/core/proto/proto_socks.c
index 78767a94ff912feb05c648826f43a435b7686b91..e131ac9e5910fb444fb432d68acea9d25b9c5b79 100644 (file)
--- a/src/core/proto/proto_socks.c
+++ b/src/core/proto/proto_socks.c
@@ -186,18 +186,15 @@ parse_socks4_request(const uint8_t *raw_data, socks_request_t *req,
   }
 
   if (*is_socks4a) {
-    // We cannot rely on trunnel here, as we want to detect if
-    // we have abnormally long hostname field.
-    const char *hostname = (char *)raw_data + SOCKS4_NETWORK_LEN +
-     usernamelen + 1;
-    size_t hostname_len = (char *)raw_data + datalen - hostname;
-
-    if (hostname_len <= sizeof(req->address)) {
-      const char *trunnel_hostname =
+    const char *trunnel_hostname =
       socks4_client_request_get_socks4a_addr_hostname(trunnel_req);
-
-      if (trunnel_hostname)
-        strlcpy(req->address, trunnel_hostname, sizeof(req->address));
+    if (BUG(!trunnel_hostname)) {
+      res = SOCKS_RESULT_INVALID;
+      goto end;
+    }
+    size_t hostname_len = strlen(trunnel_hostname);
+    if (hostname_len < sizeof(req->address)) {
+      strlcpy(req->address, trunnel_hostname, sizeof(req->address));
     } else {
       log_warn(LD_APP, "socks4: Destaddr too long. Rejecting.");
       res = SOCKS_RESULT_INVALID;