Merge remote-tracking branch 'public/bug3683'

4 files changed, 69 insertions, 17 deletions

diff --git a/src/or/buffers.c b/src/or/buffers.c
index 5b9e55ebd5..488289c06d 100644
--- a/src/or/buffers.c
+++ b/src/or/buffers.c
@@ -1532,8 +1532,14 @@ socks_request_free(socks_request_t *req)
 {
   if (!req)
     return;
-  tor_free(req->username);
-  tor_free(req->password);
+  if (req->username) {
+    memset(req->username, 0x10, req->usernamelen);
+    tor_free(req->username);
+  }
+  if (req->password) {
+    memset(req->password, 0x04, req->passwordlen);
+    tor_free(req->password);
+  }
   memset(req, 0xCC, sizeof(socks_request_t));
   tor_free(req);
 }
diff --git a/src/or/circuitlist.c b/src/or/circuitlist.c
index 48c5afc7d0..2222a25af0 100644
--- a/src/or/circuitlist.c
+++ b/src/or/circuitlist.c
@@ -552,8 +552,14 @@ circuit_free(circuit_t *circ)
     rend_data_free(ocirc->rend_data);
 
     tor_free(ocirc->dest_address);
-    tor_free(ocirc->socks_username);
-    tor_free(ocirc->socks_password);
+    if (ocirc->socks_username) {
+      memset(ocirc->socks_username, 0x12, ocirc->socks_username_len);
+      tor_free(ocirc->socks_username);
+    }
+    if (ocirc->socks_password) {
+      memset(ocirc->socks_password, 0x06, ocirc->socks_password_len);
+      tor_free(ocirc->socks_password);
+    }
   } else {
     or_circuit_t *ocirc = TO_OR_CIRCUIT(circ);
     /* Remember cell statistics for this circuit before deallocating. */
diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c
index 8f550cf66a..ae2dfd2e3a 100644
--- a/src/or/connection_edge.c
+++ b/src/or/connection_edge.c
@@ -3329,6 +3329,23 @@ parse_extended_hostname(char *address, int allowdotexit)
     return BAD_HOSTNAME;
 }
 
+/** Return true iff the (possibly NULL) <b>alen</b>-byte chunk of memory at
+ * <b>a</b> is equal to the (possibly NULL) <b>blen</b>-byte chunk of memory
+ * at <b>b</b>. */
+static int
+memeq_opt(const char *a, size_t alen, const char *b, size_t blen)
+{
+  if (a == NULL) {
+    return (b == NULL);
+  } else if (b == NULL) {
+    return 0;
+  } else if (alen != blen) {
+    return 0;
+  } else {
+    return tor_memeq(a, b, alen);
+  }
+}
+
 /** Return true iff <b>a</b> and <b>b</b> have isolation rules and fields that
  * make it permissible to put them on the same circuit.*/
 int
@@ -3336,6 +3353,8 @@ connection_edge_streams_are_compatible(const edge_connection_t *a,
                                        const edge_connection_t *b)
 {
   const uint8_t iso = a->isolation_flags | b->isolation_flags;
+  const socks_request_t *a_sr = a->socks_request;
+  const socks_request_t *b_sr = b->socks_request;
 
   if (! a->original_dest_address) {
     log_warn(LD_BUG, "Reached connection_edge_streams_are_compatible without "
@@ -3359,8 +3378,10 @@ connection_edge_streams_are_compatible(const edge_connection_t *a,
       strcasecmp(a->original_dest_address, b->original_dest_address))
     return 0;
   if ((iso & ISO_SOCKSAUTH) &&
-      (strcmp_opt(a->socks_request->username, b->socks_request->username) ||
-       strcmp_opt(a->socks_request->password, b->socks_request->password)))
+      (! memeq_opt(a_sr->username, a_sr->usernamelen,
+                   b_sr->username, b_sr->usernamelen) ||
+       ! memeq_opt(a_sr->password, a_sr->passwordlen,
+                   b_sr->password, b_sr->passwordlen)))
     return 0;
   if ((iso & ISO_CLIENTPROTO) &&
       (a->socks_request->listener_type != b->socks_request->listener_type ||
@@ -3386,6 +3407,7 @@ connection_edge_compatible_with_circuit(const edge_connection_t *conn,
                                         const origin_circuit_t *circ)
 {
   const uint8_t iso = conn->isolation_flags;
+  const socks_request_t *sr = conn->socks_request;
 
   /* If circ has never been used for an isolated connection, we can
    * totally use it for this one. */
@@ -3421,8 +3443,10 @@ connection_edge_compatible_with_circuit(const edge_connection_t *conn,
       strcasecmp(conn->original_dest_address, circ->dest_address))
     return 0;
   if ((iso & ISO_SOCKSAUTH) &&
-      (strcmp_opt(conn->socks_request->username, circ->socks_username) ||
-       strcmp_opt(conn->socks_request->password, circ->socks_password)))
+      (! memeq_opt(sr->username, sr->usernamelen,
+                   circ->socks_username, circ->socks_username_len) ||
+       ! memeq_opt(sr->password, sr->passwordlen,
+                   circ->socks_password, circ->socks_password_len)))
     return 0;
   if ((iso & ISO_CLIENTPROTO) &&
       (conn->socks_request->listener_type != circ->client_proto_type ||
@@ -3452,6 +3476,7 @@ connection_edge_update_circuit_isolation(const edge_connection_t *conn,
                                          origin_circuit_t *circ,
                                          int dry_run)
 {
+  const socks_request_t *sr = conn->socks_request;
   if (! conn->original_dest_address) {
     log_warn(LD_BUG, "Reached connection_update_circuit_isolation without "
              "having set conn->original_dest_address");
@@ -3469,10 +3494,12 @@ connection_edge_update_circuit_isolation(const edge_connection_t *conn,
     tor_addr_copy(&circ->client_addr, &TO_CONN(conn)->addr);
     circ->session_group = conn->session_group;
     circ->nym_epoch = conn->nym_epoch;
-    circ->socks_username = conn->socks_request->username ?
-      tor_strdup(conn->socks_request->username) : NULL;
-    circ->socks_password = conn->socks_request->password ?
-      tor_strdup(conn->socks_request->password) : NULL;
+    circ->socks_username = sr->username ?
+      tor_memdup(sr->username, sr->usernamelen) : NULL;
+    circ->socks_password = sr->password ?
+      tor_memdup(sr->password, sr->passwordlen) : NULL;
+    circ->socks_username_len = sr->usernamelen;
+    circ->socks_password_len = sr->passwordlen;
 
     circ->isolation_values_set = 1;
     return 0;
@@ -3482,8 +3509,10 @@ connection_edge_update_circuit_isolation(const edge_connection_t *conn,
       mixed |= ISO_DESTPORT;
     if (strcasecmp(conn->original_dest_address, circ->dest_address))
       mixed |= ISO_DESTADDR;
-    if (strcmp_opt(conn->socks_request->username, circ->socks_username) ||
-        strcmp_opt(conn->socks_request->password, circ->socks_password))
+    if (!memeq_opt(sr->username, sr->usernamelen,
+                   circ->socks_username, circ->socks_username_len) ||
+        !memeq_opt(sr->password, sr->passwordlen,
+                   circ->socks_password, circ->socks_password_len))
       mixed |= ISO_SOCKSAUTH;
     if ((conn->socks_request->listener_type != circ->client_proto_type ||
          conn->socks_request->socks_version != circ->client_proto_socksver))
@@ -3540,7 +3569,14 @@ circuit_clear_isolation(origin_circuit_t *circ)
   tor_free(circ->dest_address);
   circ->session_group = -1;
   circ->nym_epoch = 0;
-  tor_free(circ->socks_username);
-  tor_free(circ->socks_password);
+  if (circ->socks_username) {
+    memset(circ->socks_username, 0x11, circ->socks_username_len);
+    tor_free(circ->socks_username);
+  }
+  if (circ->socks_password) {
+    memset(circ->socks_password, 0x05, circ->socks_password_len);
+    tor_free(circ->socks_password);
+  }
+  circ->socks_username_len = circ->socks_password_len = 0;
 }
 
diff --git a/src/or/or.h b/src/or/or.h
index 150971b5fb..eb271b5ee4 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -2524,6 +2524,10 @@ typedef struct origin_circuit_t {
   char *dest_address;
   int session_group;
   unsigned nym_epoch;
+  size_t socks_username_len;
+  uint8_t socks_password_len;
+  /* Note that the next two values are NOT NUL-terminated; see
+     socks_username_len and socks_password_len for their lengths. */
   char *socks_username;
   char *socks_password;
   /**@}*/
@@ -3400,7 +3404,7 @@ struct socks_request_t {
   unsigned int got_auth : 1; /**< Have we received any authentication data? */
 
   /** Number of bytes in username; 0 if username is NULL */
-  uint8_t usernamelen;
+  size_t usernamelen;
   /** Number of bytes in password; 0 if password is NULL */
   uint8_t passwordlen;
   /** The negotiated username value if any (for socks5), or the entire