r17639@catbus: nickm | 2008-01-15 19:09:21 -0500

 Fix some hard to trigger but nonetheless real memory leaks spotted by an anonymous contributor.  Needs review.  Partial backport candidate.


svn:r13147

13 files changed, 46 insertions, 16 deletions

diff --git a/ChangeLog b/ChangeLog
index 7d6859e36..6cc17d1ac 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -58,6 +58,12 @@ Changes in version 0.2.0.16-alpha - 2008-01-??
       but client versions are not.
     - Fix rare bug on REDIRECTSTREAM control command when called with no
       port set: it could erroneously report an error when none had happened.
+    - Avoid bogus crash-prone, leak-prone tor_realloc when we're compressing
+      large objects and find ourselves with more than 4k left over.  Bugfix
+      on 0.2.0.
+    - Fix a small memory leak when setting up a hidden service.
+    - Fix a few memory leaks that could in theory happen under bizarre error
+      conditions.
 
   o Minor features (controller):
     - Get NS events working again.  (Patch from tup)
diff --git a/src/common/torgzip.c b/src/common/torgzip.c
index 1fc301260..09e22bd2d 100644
--- a/src/common/torgzip.c
+++ b/src/common/torgzip.c
@@ -139,7 +139,7 @@ tor_gzip_compress(char **out, size_t *out_len,
   *out_len = stream->total_out;
   if (stream->total_out > out_size + 4097) {
     /* If we're wasting more than 4k, don't. */
-    tor_realloc(*out, stream->total_out + 1);
+    *out = tor_realloc(*out, stream->total_out + 1);
   }
   if (deflateEnd(stream)!=Z_OK) {
     log_warn(LD_BUG, "Error freeing gzip structures");
diff --git a/src/common/tortls.c b/src/common/tortls.c
index 68e9baf33..8c3163389 100644
--- a/src/common/tortls.c
+++ b/src/common/tortls.c
@@ -43,7 +43,7 @@ const char tortls_c_id[] =
 #include <string.h>
 
 // #define V2_HANDSHAKE_SERVER
-// #define V2_HANDSHAKE_CLIENT
+#define V2_HANDSHAKE_CLIENT
 
 /* Copied from or.h */
 #define LEGAL_NICKNAME_CHARACTERS \
diff --git a/src/or/config.c b/src/or/config.c
index 8b34dec15..18264a4c4 100644
--- a/src/or/config.c
+++ b/src/or/config.c
@@ -1153,6 +1153,8 @@ options_act(or_options_t *old_options)
       if (parse_redirect_line(sl, cl, &errmsg)<0) {
         log_warn(LD_CONFIG, "%s", errmsg);
         tor_free(errmsg);
+        SMARTLIST_FOREACH(sl, exit_redirect_t *, er, tor_free(er));
+        smartlist_free(sl);
         return -1;
       }
     }
@@ -2096,6 +2098,7 @@ list_torrc_options(void)
       smartlist_clear(lines);
     }
   }
+  smartlist_free(lines);
 }
 
 /** Last value actually set by resolve_my_address. */
diff --git a/src/or/control.c b/src/or/control.c
index d6424d858..14ad57a1d 100644
--- a/src/or/control.c
+++ b/src/or/control.c
@@ -704,6 +704,7 @@ control_setconf_helper(control_connection_t *conn, uint32_t len, char *body,
           connection_write_str_to_buf("551 Couldn't parse string\r\n", conn);
           SMARTLIST_FOREACH(entries, char *, cp, tor_free(cp));
           smartlist_free(entries);
+          tor_free(key);
           return 0;
         }
       }
@@ -1440,6 +1441,7 @@ getinfo_helper_dir(control_connection_t *control_conn,
     res = dirserv_get_routerdescs(descs, url, &msg);
     if (res) {
       log_warn(LD_CONTROL, "getinfo '%s': %s", question, msg);
+      smartlist_free(descs);
       return -1;
     }
     SMARTLIST_FOREACH(descs, signed_descriptor_t *, sd,
diff --git a/src/or/networkstatus.c b/src/or/networkstatus.c
index b40616602..e0b07518a 100644
--- a/src/or/networkstatus.c
+++ b/src/or/networkstatus.c
@@ -1796,7 +1796,7 @@ networkstatus_getinfo_by_purpose(const char *purpose_string, time_t now)
   time_t cutoff = now - ROUTER_MAX_AGE_TO_PUBLISH;
   char *answer;
   routerlist_t *rl = router_get_routerlist();
-  smartlist_t *statuses = smartlist_create();
+  smartlist_t *statuses;
   uint8_t purpose = router_purpose_from_string(purpose_string);
   routerstatus_t rs;
   int bridge_auth = authdir_mode_bridge(get_options());
@@ -1807,6 +1807,7 @@ networkstatus_getinfo_by_purpose(const char *purpose_string, time_t now)
     return NULL;
   }
 
+  statuses = smartlist_create();
   SMARTLIST_FOREACH(rl->routers, routerinfo_t *, ri, {
     if (ri->cache_info.published_on < cutoff)
       continue;
diff --git a/src/or/onion.c b/src/or/onion.c
index aa42ab6c6..fb516da24 100644
--- a/src/or/onion.c
+++ b/src/or/onion.c
@@ -336,14 +336,13 @@ onion_skin_client_handshake(crypto_dh_env_t *handshake_state,
   len = crypto_dh_compute_secret(handshake_state, handshake_reply, DH_KEY_LEN,
                                  key_material, 20+key_out_len);
   if (len < 0)
-    return -1;
+    goto err;
 
   if (memcmp(key_material, handshake_reply+DH_KEY_LEN, 20)) {
     /* H(K) does *not* match. Something fishy. */
-    tor_free(key_material);
     log_warn(LD_PROTOCOL,"Digest DOES NOT MATCH on onion handshake. "
              "Bug or attack.");
-    return -1;
+    goto err;
   }
 
   /* use the rest of the key material for our shared keys, digests, etc */
@@ -357,6 +356,9 @@ onion_skin_client_handshake(crypto_dh_env_t *handshake_state,
 
   tor_free(key_material);
   return 0;
+ err:
+  tor_free(key_material);
+  return -1;
 }
 
 /** Implement the server side of the CREATE_FAST abbreviated handshake.  The
@@ -429,6 +431,7 @@ fast_client_handshake(const char *handshake_state, /* DIGEST_LEN bytes */
     /* H(K) does *not* match. Something fishy. */
     log_warn(LD_PROTOCOL,"Digest DOES NOT MATCH on fast handshake. "
              "Bug or attack.");
+    tor_free(out);
     return -1;
   }
   memcpy(key_out, out+DIGEST_LEN, key_out_len);
diff --git a/src/or/rendservice.c b/src/or/rendservice.c
index 60bced974..1a355c883 100644
--- a/src/or/rendservice.c
+++ b/src/or/rendservice.c
@@ -284,6 +284,7 @@ rend_config_services(or_options_t *options, int validate_only)
         log_warn(LD_CONFIG,
                  "Got multiple HiddenServiceNodes lines for a single "
                  "service.");
+        rend_service_free(service);
         return -1;
       }
       service->intro_prefer_nodes = tor_strdup(line->value);
@@ -292,6 +293,7 @@ rend_config_services(or_options_t *options, int validate_only)
         log_warn(LD_CONFIG,
                  "Got multiple HiddenServiceExcludedNodes lines for "
                  "a single service.");
+        rend_service_free(service);
         return -1;
       }
       service->intro_exclude_nodes = tor_strdup(line->value);
@@ -308,6 +310,9 @@ rend_config_services(or_options_t *options, int validate_only)
         if (strlen(version_str) != 1 || strspn(version_str, "02") != 1) {
           log_warn(LD_CONFIG,
                    "HiddenServiceVersion can only be 0 and/or 2.");
+          SMARTLIST_FOREACH(versions, char *, cp, tor_free(cp));
+          smartlist_free(versions);
+          rend_service_free(service);
           return -1;
         }
         version = atoi(version_str);
@@ -317,6 +322,8 @@ rend_config_services(or_options_t *options, int validate_only)
        * value; otherwise leave it at -1. */
       if (versions_bitmask == 1 << 0) service->descriptor_version = 0;
       if (versions_bitmask == 1 << 2) service->descriptor_version = 2;
+      SMARTLIST_FOREACH(versions, char *, cp, tor_free(cp));
+      smartlist_free(versions);
     }
   }
   if (service) {
@@ -618,7 +625,7 @@ rend_service_introduce(origin_circuit_t *circuit, const char *request,
   if (len != REND_COOKIE_LEN+DH_KEY_LEN) {
     log_warn(LD_PROTOCOL, "Bad length %u for INTRODUCE2 cell.", (int)len);
     reason = END_CIRC_REASON_TORPROTOCOL;
-    return -1;
+    goto err;
   }
 
   r_cookie = ptr;
diff --git a/src/or/router.c b/src/or/router.c
index 9d4c3fdd1..8851fb0d3 100644
--- a/src/or/router.c
+++ b/src/or/router.c
@@ -397,8 +397,10 @@ init_keys(void)
   if (!server_mode(options)) {
     if (!(prkey = crypto_new_pk_env()))
       return -1;
-    if (crypto_pk_generate_key(prkey))
+    if (crypto_pk_generate_key(prkey)) {
+      crypto_free_pk_env(prkey);
       return -1;
+    }
     set_identity_key(prkey);
     /* Create a TLS context; default the client nickname to "client". */
     if (tor_tls_context_new(get_identity_key(),
@@ -1302,6 +1304,7 @@ router_rebuild_descriptor(int force)
   if (extrainfo_dump_to_string(ei->cache_info.signed_descriptor_body, 8192,
                                ei, get_identity_key()) < 0) {
     log_warn(LD_BUG, "Couldn't generate extra-info descriptor.");
+    extrainfo_free(ei);
     return -1;
   }
   ei->cache_info.signed_descriptor_len =
diff --git a/src/or/routerlist.c b/src/or/routerlist.c
index 1c48c762a..5a16062f5 100644
--- a/src/or/routerlist.c
+++ b/src/or/routerlist.c
@@ -961,14 +961,14 @@ router_pick_trusteddirserver_impl(authority_type_t type, int flags)
   int fascistfirewall = ! (flags & PDS_IGNORE_FASCISTFIREWALL);
   int prefer_tunnel = (flags & _PDS_PREFER_TUNNELED_DIR_CONNS);
 
+  if (!trusted_dir_servers)
+    return NULL;
+
   direct = smartlist_create();
   tunnel = smartlist_create();
   overloaded_direct = smartlist_create();
   overloaded_tunnel = smartlist_create();
 
-  if (!trusted_dir_servers)
-    return NULL;
-
   SMARTLIST_FOREACH(trusted_dir_servers, trusted_dir_server_t *, d,
     {
       int is_overloaded =
@@ -2006,8 +2006,10 @@ signed_descriptor_get_body_impl(signed_descriptor_t *desc,
   tor_assert(r);
   if (!with_annotations) {
     if (memcmp("router ", r, 7) && memcmp("extra-info ", r, 11)) {
+      char *cp = tor_strndup(r, 64);
       log_err(LD_DIR, "descriptor at %p begins with unexpected string %s",
-              desc, tor_strndup(r, 64));
+              desc, escaped(cp));
+      tor_free(cp);
     }
     tor_assert(!memcmp("router ", r, 7) || !memcmp("extra-info ", r, 11));
   }
diff --git a/src/or/routerparse.c b/src/or/routerparse.c
index 4b0d65219..a7c54f9ff 100644
--- a/src/or/routerparse.c
+++ b/src/or/routerparse.c
@@ -789,7 +789,7 @@ find_dir_signing_key(const char *str, const char *eos)
   }
   if (tok->tp != K_DIR_SIGNING_KEY) {
     log_warn(LD_DIR, "Dir-signing-key token did not parse as expected");
-    return NULL;
+    goto done;
   }
 
   if (tok->key) {
@@ -797,10 +797,10 @@ find_dir_signing_key(const char *str, const char *eos)
     tok->key = NULL; /* steal reference. */
   } else {
     log_warn(LD_DIR, "Dir-signing-key token contained no key");
-    return NULL;
   }
 
-  token_free(tok);
+ done:
+  if (tok) token_free(tok);
   return key;
 }
 
@@ -1090,7 +1090,7 @@ router_parse_entry_from_string(const char *s, const char *end,
 
   if (router_get_router_hash(s, digest) < 0) {
     log_warn(LD_DIR, "Couldn't compute router hash.");
-    return NULL;
+    goto err;
   }
   {
     int flags = 0;
diff --git a/src/tools/tor-gencert.c b/src/tools/tor-gencert.c
index ef028891f..5d23b5782 100644
--- a/src/tools/tor-gencert.c
+++ b/src/tools/tor-gencert.c
@@ -370,6 +370,8 @@ generate_certificate(void)
                "dir-key-certification\n",
                address?"\ndir-address ":"", address?address:"",
                fingerprint, published, expires, ident, signing);
+  tor_free(ident);
+  tor_free(signing);
   signed_len = strlen(buf);
   SHA1((const unsigned char*)buf,signed_len,(unsigned char*)digest);
 
diff --git a/src/tools/tor-resolve.c b/src/tools/tor-resolve.c
index 23c1bf005..0269de88f 100644
--- a/src/tools/tor-resolve.c
+++ b/src/tools/tor-resolve.c
@@ -195,6 +195,7 @@ do_resolve(const char *hostname, uint32_t sockshost, uint16_t socksport,
   if ((len = build_socks_resolve_request(&req, "", hostname, reverse,
                                          version))<0) {
     log_err(LD_BUG,"Error generating SOCKS request");
+    tor_assert(!req);
     return -1;
   }
   if (write_all(s, req, len, 1) != len) {