commit 9f044eac77ee2245de71283e71361346ee194f25 Author: Nick Mathewson nickm@torproject.org Date: Wed Feb 20 00:36:59 2013 -0500

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. --- src/or/dirvote.c | 138 +++++++++++++++++++----------------------------------- 1 file changed, 47 insertions(+), 91 deletions(-)

diff --git a/src/or/dirvote.c b/src/or/dirvote.c index bcfe2b0..bd4e2f6 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; }