Refactor format_networkstatus_vote to avoid preallocating a buffer.

This saves a lot of "are we about to overrun the buffer?" checking,
and unmoots a bunch of "did we allocate enough" discussion.

1 files changed, 47 insertions, 91 deletions

diff --git a/src/or/dirvote.c b/src/or/dirvote.c
index bcfe2b069..bd4e2f69a 100644
--- a/src/or/dirvote.c
+++ b/src/or/dirvote.c
@@ -67,17 +67,15 @@ char *
 format_networkstatus_vote(crypto_pk_t *private_signing_key,
                           networkstatus_t *v3_ns)
 {
-  size_t len;
-  char *status = NULL;
+  smartlist_t *chunks;
   const char *client_versions = NULL, *server_versions = NULL;
-  char *outp, *endp;
   char fingerprint[FINGERPRINT_LEN+1];
   char digest[DIGEST_LEN];
   uint32_t addr;
-  routerlist_t *rl = router_get_routerlist();
-  char *version_lines = NULL;
-  int r;
+  char *client_versions_line = NULL, *server_versions_line = NULL;
   networkstatus_voter_info_t *voter;
+  char *status = NULL;
+  size_t status_len;
 
   tor_assert(private_signing_key);
   tor_assert(v3_ns->type == NS_TYPE_VOTE || v3_ns->type == NS_TYPE_OPINION);
@@ -91,43 +89,20 @@ format_networkstatus_vote(crypto_pk_t *private_signing_key,
   client_versions = v3_ns->client_versions;
   server_versions = v3_ns->server_versions;
 
-  if (client_versions || server_versions) {
-    size_t v_len = 64;
-    char *cp;
-    if (client_versions)
-      v_len += strlen(client_versions);
-    if (server_versions)
-      v_len += strlen(server_versions);
-    version_lines = tor_malloc(v_len);
-    cp = version_lines;
-    if (client_versions) {
-      r = tor_snprintf(cp, v_len-(cp-version_lines),
-                   "client-versions %s\n", client_versions);
-      if (r < 0) {
-        log_err(LD_BUG, "Insufficient memory for client-versions line");
-        tor_assert(0);
-      }
-      cp += strlen(cp);
-    }
-    if (server_versions) {
-      r = tor_snprintf(cp, v_len-(cp-version_lines),
-                   "server-versions %s\n", server_versions);
-      if (r < 0) {
-        log_err(LD_BUG, "Insufficient memory for server-versions line");
-        tor_assert(0);
-      }
-    }
+  if (client_versions) {
+    tor_asprintf(&client_versions_line, "client-versions %s\n",
+                 client_versions);
   } else {
-    version_lines = tor_strdup("");
+    client_versions_line = tor_strdup("");
+  }
+  if (server_versions) {
+    tor_asprintf(&server_versions_line, "server-versions %s\n",
+                 server_versions);
+  } else {
+    server_versions_line = tor_strdup("");
   }
 
-  len = 8192;
-  len += strlen(version_lines);
-  len += (RS_ENTRY_LEN+MICRODESC_LINE_LEN)*smartlist_len(rl->routers);
-  len += strlen("\ndirectory-footer\n");
-  len += v3_ns->cert->cache_info.signed_descriptor_len;
-
-  status = tor_malloc(len);
+  chunks = smartlist_new();
   {
     char published[ISO_TIME_LEN+1];
     char va[ISO_TIME_LEN+1];
@@ -151,7 +126,7 @@ format_networkstatus_vote(crypto_pk_t *private_signing_key,
       params = tor_strdup("");
 
     tor_assert(cert);
-    r = tor_snprintf(status, len,
+    smartlist_add_asprintf(chunks,
                  "network-status-version 3\n"
                  "vote-status %s\n"
                  "consensus-methods %s\n"
@@ -160,7 +135,7 @@ format_networkstatus_vote(crypto_pk_t *private_signing_key,
                  "fresh-until %s\n"
                  "valid-until %s\n"
                  "voting-delay %d %d\n"
-                 "%s" /* versions */
+                 "%s%s" /* versions */
                  "known-flags %s\n"
                  "flag-thresholds %s\n"
                  "params %s\n"
@@ -170,7 +145,8 @@ format_networkstatus_vote(crypto_pk_t *private_signing_key,
                  methods,
                  published, va, fu, vu,
                  v3_ns->vote_seconds, v3_ns->dist_seconds,
-                 version_lines,
+                 client_versions_line,
+                 server_versions_line,
                  flags,
                  flag_thresholds,
                  params,
@@ -178,88 +154,63 @@ format_networkstatus_vote(crypto_pk_t *private_signing_key,
                  fmt_addr32(addr), voter->dir_port, voter->or_port,
                  voter->contact);
 
-    if (r < 0) {
-      log_err(LD_BUG, "Insufficient memory for network status line");
-      tor_assert(0);
-    }
-
     tor_free(params);
     tor_free(flags);
     tor_free(flag_thresholds);
     tor_free(methods);
-    outp = status + strlen(status);
-    endp = status + len;
 
     if (!tor_digest_is_zero(voter->legacy_id_digest)) {
       char fpbuf[HEX_DIGEST_LEN+1];
       base16_encode(fpbuf, sizeof(fpbuf), voter->legacy_id_digest, DIGEST_LEN);
-      r = tor_snprintf(outp, endp-outp, "legacy-dir-key %s\n", fpbuf);
-      if (r < 0) {
-        log_err(LD_BUG, "Insufficient memory for legacy-dir-key line");
-        tor_assert(0);
-      }
-      outp += strlen(outp);
+      smartlist_add_asprintf(chunks, "legacy-dir-key %s\n", fpbuf);
     }
 
-    tor_assert(outp + cert->cache_info.signed_descriptor_len < endp);
-    memcpy(outp, cert->cache_info.signed_descriptor_body,
-           cert->cache_info.signed_descriptor_len);
-
-    outp += cert->cache_info.signed_descriptor_len;
+    smartlist_add(chunks, tor_strndup(cert->cache_info.signed_descriptor_body,
+                                      cert->cache_info.signed_descriptor_len));
   }
 
   SMARTLIST_FOREACH_BEGIN(v3_ns->routerstatus_list, vote_routerstatus_t *,
                           vrs) {
+#define MAX_VOTE_ROUTERSTATUS_LEN 8192
+    char rs_buf[MAX_VOTE_ROUTERSTATUS_LEN];
     vote_microdesc_hash_t *h;
-    if (routerstatus_format_entry(outp, endp-outp, &vrs->status,
+    if (routerstatus_format_entry(rs_buf, sizeof(rs_buf), &vrs->status,
                                   vrs->version, NS_V3_VOTE, vrs) < 0) {
-      log_warn(LD_BUG, "Unable to print router status.");
-      goto err;
+      log_warn(LD_BUG, "Unable to print router status; skipping");
+      continue;
     }
-    outp += strlen(outp);
+    smartlist_add(chunks, tor_strdup(rs_buf));
 
     for (h = vrs->microdesc; h; h = h->next) {
-      size_t mlen = strlen(h->microdesc_hash_line);
-      if (outp+mlen >= endp) {
-        log_warn(LD_BUG, "Can't fit microdesc line in vote.");
-      }
-      memcpy(outp, h->microdesc_hash_line, mlen+1);
-      outp += strlen(outp);
+      smartlist_add(chunks, tor_strdup(h->microdesc_hash_line));
     }
   } SMARTLIST_FOREACH_END(vrs);
 
-  r = tor_snprintf(outp, endp-outp, "directory-footer\n");
-  if (r < 0) {
-    log_err(LD_BUG, "Insufficient memory for directory-footer line");
-    tor_assert(0);
-  }
-  outp += strlen(outp);
+  smartlist_add(chunks, tor_strdup("directory-footer\n"));
 
   {
     char signing_key_fingerprint[FINGERPRINT_LEN+1];
-    if (tor_snprintf(outp, endp-outp, "directory-signature ")<0) {
-      log_warn(LD_BUG, "Unable to start signature line.");
-      goto err;
-    }
-    outp += strlen(outp);
-
     if (crypto_pk_get_fingerprint(private_signing_key,
                                   signing_key_fingerprint, 0)<0) {
       log_warn(LD_BUG, "Unable to get fingerprint for signing key");
       goto err;
     }
-    if (tor_snprintf(outp, endp-outp, "%s %s\n", fingerprint,
-                     signing_key_fingerprint)<0) {
-      log_warn(LD_BUG, "Unable to end signature line.");
-      goto err;
-    }
-    outp += strlen(outp);
+
+    smartlist_add_asprintf(chunks, "directory-signature %s %s\n", fingerprint,
+                           signing_key_fingerprint);
   }
 
+  status = smartlist_join_strings(chunks, "", 0, NULL);
+#define MAX_VOTE_SIGNATURE_LEN 4096
+  status_len = strlen(status) + MAX_VOTE_SIGNATURE_LEN + 1;
+  status = tor_realloc(status, status_len);
+
   if (router_get_networkstatus_v3_hash(status, digest, DIGEST_SHA1)<0)
     goto err;
   note_crypto_pk_op(SIGN_DIR);
-  if (router_append_dirobj_signature(outp,endp-outp,digest, DIGEST_LEN,
+  if (router_append_dirobj_signature(status+strlen(status),
+                                     status_len,
+                                     digest, DIGEST_LEN,
                                      private_signing_key)<0) {
     log_warn(LD_BUG, "Unable to sign networkstatus vote.");
     goto err;
@@ -282,7 +233,12 @@ format_networkstatus_vote(crypto_pk_t *private_signing_key,
  err:
   tor_free(status);
  done:
-  tor_free(version_lines);
+  tor_free(client_versions_line);
+  tor_free(server_versions_line);
+  if (chunks) {
+    SMARTLIST_FOREACH(chunks, char *, cp, tor_free(cp));
+    smartlist_free(chunks);
+  }
   return status;
 }