commit abaca3fc8c6bc54408084e7514468fa2cd7b3edf Author: Nick Mathewson nickm@torproject.org Date: Tue Sep 11 10:32:17 2018 -0400
Revise networkstatus parsing code to use lengths
This way the networkstatus can be parsed without being NUL-terminated, so we can implement 27244 and mmap our consensus objects. --- src/feature/dirauth/dirvote.c | 15 ++++++++---- src/feature/nodelist/networkstatus.c | 4 +++- src/feature/nodelist/routerparse.c | 44 +++++++++++++++++++++--------------- src/feature/nodelist/routerparse.h | 6 +++-- src/test/fuzz/fuzz_consensus.c | 6 ++--- src/test/fuzz/fuzz_vrs.c | 16 ++++++------- src/test/test_dir.c | 32 ++++++++++++++++++++------ src/test/test_dir_common.c | 5 ++-- src/test/test_routerlist.c | 4 +++- 9 files changed, 85 insertions(+), 47 deletions(-)
diff --git a/src/feature/dirauth/dirvote.c b/src/feature/dirauth/dirvote.c index 14357c770..42821760c 100644 --- a/src/feature/dirauth/dirvote.c +++ b/src/feature/dirauth/dirvote.c @@ -401,7 +401,8 @@ format_networkstatus_vote(crypto_pk_t *private_signing_key,
{ networkstatus_t *v; - if (!(v = networkstatus_parse_vote_from_string(status, NULL, + if (!(v = networkstatus_parse_vote_from_string(status, strlen(status), + NULL, v3_ns->type))) { log_err(LD_BUG,"Generated a networkstatus %s we couldn't parse: " "<<%s>>", @@ -2398,7 +2399,8 @@ networkstatus_compute_consensus(smartlist_t *votes,
{ networkstatus_t *c; - if (!(c = networkstatus_parse_vote_from_string(result, NULL, + if (!(c = networkstatus_parse_vote_from_string(result, strlen(result), + NULL, NS_TYPE_CONSENSUS))) { log_err(LD_BUG, "Generated a networkstatus consensus we couldn't " "parse."); @@ -3121,7 +3123,8 @@ dirvote_add_vote(const char *vote_body, const char **msg_out, int *status_out) *msg_out = NULL;
again: - vote = networkstatus_parse_vote_from_string(vote_body, &end_of_vote, + vote = networkstatus_parse_vote_from_string(vote_body, strlen(vote_body), + &end_of_vote, NS_TYPE_VOTE); if (!end_of_vote) end_of_vote = vote_body + strlen(vote_body); @@ -3379,7 +3382,9 @@ dirvote_compute_consensuses(void) flavor_name); continue; } - consensus = networkstatus_parse_vote_from_string(consensus_body, NULL, + consensus = networkstatus_parse_vote_from_string(consensus_body, + strlen(consensus_body), + NULL, NS_TYPE_CONSENSUS); if (!consensus) { log_warn(LD_DIR, "Couldn't parse %s consensus we generated!", @@ -3518,7 +3523,7 @@ dirvote_add_signatures_to_pending_consensus( * just in case we break detached signature processing at some point. */ { networkstatus_t *v = networkstatus_parse_vote_from_string( - pc->body, NULL, + pc->body, strlen(pc->body), NULL, NS_TYPE_CONSENSUS); tor_assert(v); networkstatus_vote_free(v); diff --git a/src/feature/nodelist/networkstatus.c b/src/feature/nodelist/networkstatus.c index 6492b828b..4338fde59 100644 --- a/src/feature/nodelist/networkstatus.c +++ b/src/feature/nodelist/networkstatus.c @@ -1861,7 +1861,9 @@ networkstatus_set_current_consensus(const char *consensus, }
/* Make sure it's parseable. */ - c = networkstatus_parse_vote_from_string(consensus, NULL, NS_TYPE_CONSENSUS); + c = networkstatus_parse_vote_from_string(consensus, + strlen(consensus), + NULL, NS_TYPE_CONSENSUS); if (!c) { log_warn(LD_DIR, "Unable to parse networkstatus consensus"); result = -2; diff --git a/src/feature/nodelist/routerparse.c b/src/feature/nodelist/routerparse.c index 9c51799d9..9abdfb614 100644 --- a/src/feature/nodelist/routerparse.c +++ b/src/feature/nodelist/routerparse.c @@ -1057,9 +1057,10 @@ router_get_networkstatus_v3_sha3_as_signed(uint8_t *digest_out, /** Set <b>digests</b> to all the digests of the consensus document in * <b>s</b> */ int -router_get_networkstatus_v3_hashes(const char *s, common_digests_t *digests) +router_get_networkstatus_v3_hashes(const char *s, size_t len, + common_digests_t *digests) { - return router_get_hashes_impl(s,strlen(s),digests, + return router_get_hashes_impl(s, len, digests, "network-status-version", "\ndirectory-signature", ' '); @@ -2489,18 +2490,19 @@ authority_cert_parse_from_string(const char *s, const char **end_of_string) return NULL; }
-/** Helper: given a string <b>s</b>, return the start of the next router-status +/** Helper: given a string <b>s</b> ending at <b>s_eos</b>, return the + * start of the next router-status * object (starting with "r " at the start of a line). If none is found, * return the start of the directory footer, or the next directory signature. * If none is found, return the end of the string. */ static inline const char * -find_start_of_next_routerstatus(const char *s) +find_start_of_next_routerstatus(const char *s, const char *s_eos) { const char *eos, *footer, *sig; - if ((eos = strstr(s, "\nr "))) + if ((eos = tor_memstr(s, s_eos - s, "\nr "))) ++eos; else - eos = s + strlen(s); + eos = s_eos;
footer = tor_memstr(s, eos-s, "\ndirectory-footer"); sig = tor_memstr(s, eos-s, "\ndirectory-signature"); @@ -2632,7 +2634,8 @@ summarize_protover_flags(protover_summary_flags_t *out, **/ STATIC routerstatus_t * routerstatus_parse_entry_from_string(memarea_t *area, - const char **s, smartlist_t *tokens, + const char **s, const char *s_eos, + smartlist_t *tokens, networkstatus_t *vote, vote_routerstatus_t *vote_rs, int consensus_method, @@ -2651,7 +2654,7 @@ routerstatus_parse_entry_from_string(memarea_t *area, flav = FLAV_NS; tor_assert(flav == FLAV_NS || flav == FLAV_MICRODESC);
- eos = find_start_of_next_routerstatus(*s); + eos = find_start_of_next_routerstatus(*s, s_eos);
if (tokenize_string(area,*s, eos, tokens, rtrstatus_token_table,0)) { log_warn(LD_DIR, "Error tokenizing router status"); @@ -3394,7 +3397,9 @@ extract_shared_random_srvs(networkstatus_t *ns, smartlist_t *tokens) /** Parse a v3 networkstatus vote, opinion, or consensus (depending on * ns_type), from <b>s</b>, and return the result. Return NULL on failure. */ networkstatus_t * -networkstatus_parse_vote_from_string(const char *s, const char **eos_out, +networkstatus_parse_vote_from_string(const char *s, + size_t s_len, + const char **eos_out, networkstatus_type_t ns_type) { smartlist_t *tokens = smartlist_new(); @@ -3410,21 +3415,22 @@ networkstatus_parse_vote_from_string(const char *s, const char **eos_out, memarea_t *area = NULL, *rs_area = NULL; consensus_flavor_t flav = FLAV_NS; char *last_kwd=NULL; + const char *eos = s + s_len;
tor_assert(s);
if (eos_out) *eos_out = NULL;
- if (router_get_networkstatus_v3_hashes(s, &ns_digests) || + if (router_get_networkstatus_v3_hashes(s, s_len, &ns_digests) || router_get_networkstatus_v3_sha3_as_signed(sha3_as_signed, - s, strlen(s))<0) { + s, s_len)<0) { log_warn(LD_DIR, "Unable to compute digest of network-status"); goto err; }
area = memarea_new(); - end_of_header = find_start_of_next_routerstatus(s); + end_of_header = find_start_of_next_routerstatus(s, eos); if (tokenize_string(area, s, end_of_header, tokens, (ns_type == NS_TYPE_CONSENSUS) ? networkstatus_consensus_token_table : @@ -3455,7 +3461,8 @@ networkstatus_parse_vote_from_string(const char *s, const char **eos_out,
if (ns_type != NS_TYPE_CONSENSUS) { const char *end_of_cert = NULL; - if (!(cert = strstr(s, "\ndir-key-certificate-version"))) + if (!(cert = tor_memstr(s, end_of_header - s, + "\ndir-key-certificate-version"))) goto err; ++cert; ns->cert = authority_cert_parse_from_string(cert, &end_of_cert); @@ -3768,10 +3775,10 @@ networkstatus_parse_vote_from_string(const char *s, const char **eos_out, s = end_of_header; ns->routerstatus_list = smartlist_new();
- while (!strcmpstart(s, "r ")) { + while (eos-s >= 2 && fast_memeq(s, "r ", 2)) { if (ns->type != NS_TYPE_CONSENSUS) { vote_routerstatus_t *rs = tor_malloc_zero(sizeof(vote_routerstatus_t)); - if (routerstatus_parse_entry_from_string(rs_area, &s, rs_tokens, ns, + if (routerstatus_parse_entry_from_string(rs_area, &s, eos, rs_tokens, ns, rs, 0, 0)) { smartlist_add(ns->routerstatus_list, rs); } else { @@ -3779,7 +3786,8 @@ networkstatus_parse_vote_from_string(const char *s, const char **eos_out, } } else { routerstatus_t *rs; - if ((rs = routerstatus_parse_entry_from_string(rs_area, &s, rs_tokens, + if ((rs = routerstatus_parse_entry_from_string(rs_area, &s, eos, + rs_tokens, NULL, NULL, ns->consensus_method, flav))) { @@ -3824,10 +3832,10 @@ networkstatus_parse_vote_from_string(const char *s, const char **eos_out,
/* Parse footer; check signature. */ footer_tokens = smartlist_new(); - if ((end_of_footer = strstr(s, "\nnetwork-status-version "))) + if ((end_of_footer = tor_memstr(s, eos-s, "\nnetwork-status-version "))) ++end_of_footer; else - end_of_footer = s + strlen(s); + end_of_footer = eos; if (tokenize_string(area,s, end_of_footer, footer_tokens, networkstatus_vote_footer_token_table, 0)) { log_warn(LD_DIR, "Error tokenizing network-status vote footer."); diff --git a/src/feature/nodelist/routerparse.h b/src/feature/nodelist/routerparse.h index be455984d..390088c94 100644 --- a/src/feature/nodelist/routerparse.h +++ b/src/feature/nodelist/routerparse.h @@ -30,7 +30,7 @@ enum networkstatus_type_t;
int router_get_router_hash(const char *s, size_t s_len, char *digest); int router_get_dir_hash(const char *s, char *digest); -int router_get_networkstatus_v3_hashes(const char *s, +int router_get_networkstatus_v3_hashes(const char *s, size_t len, common_digests_t *digests); int router_get_networkstatus_v3_signed_boundaries(const char *s, size_t len, const char **start_out, @@ -81,6 +81,7 @@ void dump_distinct_digest_count(int severity); int compare_vote_routerstatus_entries(const void **_a, const void **_b); int networkstatus_verify_bw_weights(networkstatus_t *ns, int); networkstatus_t *networkstatus_parse_vote_from_string(const char *s, + size_t len, const char **eos_out, enum networkstatus_type_t ns_type); ns_detached_signatures_t *networkstatus_parse_detached_signatures( @@ -139,7 +140,8 @@ STATIC void dump_desc_fifo_cleanup(void); struct memarea_t; STATIC routerstatus_t *routerstatus_parse_entry_from_string( struct memarea_t *area, - const char **s, smartlist_t *tokens, + const char **s, const char *eos, + smartlist_t *tokens, networkstatus_t *vote, vote_routerstatus_t *vote_rs, int consensus_method, diff --git a/src/test/fuzz/fuzz_consensus.c b/src/test/fuzz/fuzz_consensus.c index b170fd33d..5a04683a1 100644 --- a/src/test/fuzz/fuzz_consensus.c +++ b/src/test/fuzz/fuzz_consensus.c @@ -59,13 +59,13 @@ int fuzz_main(const uint8_t *data, size_t sz) { networkstatus_t *ns; - char *str = tor_memdup_nulterm(data, sz); const char *eos = NULL; networkstatus_type_t tp = NS_TYPE_CONSENSUS; if (tor_memstr(data, MIN(sz, 1024), "tus vote")) tp = NS_TYPE_VOTE; const char *what = (tp == NS_TYPE_CONSENSUS) ? "consensus" : "vote"; - ns = networkstatus_parse_vote_from_string(str, + ns = networkstatus_parse_vote_from_string((const char *)data, + sz, &eos, tp); if (ns) { @@ -74,6 +74,6 @@ fuzz_main(const uint8_t *data, size_t sz) } else { log_debug(LD_GENERAL, "Parsing as %s failed", what); } - tor_free(str); + return 0; } diff --git a/src/test/fuzz/fuzz_vrs.c b/src/test/fuzz/fuzz_vrs.c index 8c96851b1..5665ffeac 100644 --- a/src/test/fuzz/fuzz_vrs.c +++ b/src/test/fuzz/fuzz_vrs.c @@ -52,24 +52,24 @@ fuzz_cleanup(void) int fuzz_main(const uint8_t *data, size_t sz) { - char *str = tor_memdup_nulterm(data, sz); const char *s; routerstatus_t *rs_ns = NULL, *rs_md = NULL, *rs_vote = NULL; vote_routerstatus_t *vrs = tor_malloc_zero(sizeof(*vrs)); smartlist_t *tokens = smartlist_new(); + const char *eos = (const char *)data + sz;
- s = str; - rs_ns = routerstatus_parse_entry_from_string(area, &s, tokens, + s = (const char *)data; + rs_ns = routerstatus_parse_entry_from_string(area, &s, eos, tokens, NULL, NULL, 26, FLAV_NS); tor_assert(smartlist_len(tokens) == 0);
- s = str; - rs_md = routerstatus_parse_entry_from_string(area, &s, tokens, + s = (const char *)data; + rs_md = routerstatus_parse_entry_from_string(area, &s, eos, tokens, NULL, NULL, 26, FLAV_MICRODESC); tor_assert(smartlist_len(tokens) == 0);
- s = str; - rs_vote = routerstatus_parse_entry_from_string(area, &s, tokens, + s = (const char *)data; + rs_vote = routerstatus_parse_entry_from_string(area, &s, eos, tokens, dummy_vote, vrs, 26, FLAV_NS); tor_assert(smartlist_len(tokens) == 0);
@@ -81,6 +81,6 @@ fuzz_main(const uint8_t *data, size_t sz) vote_routerstatus_free(vrs); memarea_clear(area); smartlist_free(tokens); - tor_free(str); + return 0; } diff --git a/src/test/test_dir.c b/src/test/test_dir.c index 723799ee8..0fa5c3103 100644 --- a/src/test/test_dir.c +++ b/src/test/test_dir.c @@ -70,6 +70,23 @@
#define NS_MODULE dir
+static networkstatus_t * +networkstatus_parse_vote_from_string_(const char *s, + const char **eos_out, + enum networkstatus_type_t ns_type) +{ + size_t len = strlen(s); + // memdup so that it won't be nul-terminated. + char *tmp = tor_memdup(s, len); + networkstatus_t *result = + networkstatus_parse_vote_from_string(tmp, len, eos_out, ns_type); + if (eos_out && *eos_out) { + *eos_out = s + (*eos_out - tmp); + } + tor_free(tmp); + return result; +} + static void test_dir_nicknames(void *arg) { @@ -2888,7 +2905,7 @@ test_a_networkstatus( sign_skey_leg1, FLAV_NS); tt_assert(consensus_text); - con = networkstatus_parse_vote_from_string(consensus_text, NULL, + con = networkstatus_parse_vote_from_string_(consensus_text, NULL, NS_TYPE_CONSENSUS); tt_assert(con); //log_notice(LD_GENERAL, "<<%s>>\n<<%s>>\n<<%s>>\n", @@ -2900,7 +2917,7 @@ test_a_networkstatus( sign_skey_leg1, FLAV_MICRODESC); tt_assert(consensus_text_md); - con_md = networkstatus_parse_vote_from_string(consensus_text_md, NULL, + con_md = networkstatus_parse_vote_from_string_(consensus_text_md, NULL, NS_TYPE_CONSENSUS); tt_assert(con_md); tt_int_op(con_md->flavor,OP_EQ, FLAV_MICRODESC); @@ -2999,13 +3016,13 @@ test_a_networkstatus( tt_assert(consensus_text3); tt_assert(consensus_text_md2); tt_assert(consensus_text_md3); - con2 = networkstatus_parse_vote_from_string(consensus_text2, NULL, + con2 = networkstatus_parse_vote_from_string_(consensus_text2, NULL, NS_TYPE_CONSENSUS); - con3 = networkstatus_parse_vote_from_string(consensus_text3, NULL, + con3 = networkstatus_parse_vote_from_string_(consensus_text3, NULL, NS_TYPE_CONSENSUS); - con_md2 = networkstatus_parse_vote_from_string(consensus_text_md2, NULL, + con_md2 = networkstatus_parse_vote_from_string_(consensus_text_md2, NULL, NS_TYPE_CONSENSUS); - con_md3 = networkstatus_parse_vote_from_string(consensus_text_md3, NULL, + con_md3 = networkstatus_parse_vote_from_string_(consensus_text_md3, NULL, NS_TYPE_CONSENSUS); tt_assert(con2); tt_assert(con3); @@ -6020,9 +6037,10 @@ test_dir_assumed_flags(void *arg) "192.168.0.1 9001 0\n" "m thisoneislongerbecauseitisa256bitmddigest33\n" "s Fast Guard Stable\n"; + const char *eos = str1 + strlen(str1);
const char *cp = str1; - rs = routerstatus_parse_entry_from_string(area, &cp, tokens, NULL, NULL, + rs = routerstatus_parse_entry_from_string(area, &cp, eos, tokens, NULL, NULL, 24, FLAV_MICRODESC); tt_assert(rs); tt_assert(rs->is_flagged_running); diff --git a/src/test/test_dir_common.c b/src/test/test_dir_common.c index e65e2b011..6e3bb9945 100644 --- a/src/test/test_dir_common.c +++ b/src/test/test_dir_common.c @@ -264,7 +264,9 @@ dir_common_add_rs_and_parse(networkstatus_t *vote, networkstatus_t **vote_out, /* dump the vote and try to parse it. */ v_text = format_networkstatus_vote(sign_skey, vote); tt_assert(v_text); - *vote_out = networkstatus_parse_vote_from_string(v_text, NULL, NS_TYPE_VOTE); + *vote_out = networkstatus_parse_vote_from_string(v_text, + strlen(v_text), + NULL, NS_TYPE_VOTE);
done: if (v_text) @@ -422,4 +424,3 @@ dir_common_construct_vote_3(networkstatus_t **vote, authority_cert_t *cert,
return 0; } - diff --git a/src/test/test_routerlist.c b/src/test/test_routerlist.c index 89d1f4f90..7fe4c15b1 100644 --- a/src/test/test_routerlist.c +++ b/src/test/test_routerlist.c @@ -270,7 +270,9 @@ test_router_pick_directory_server_impl(void *arg)
construct_consensus(&consensus_text_md, now); tt_assert(consensus_text_md); - con_md = networkstatus_parse_vote_from_string(consensus_text_md, NULL, + con_md = networkstatus_parse_vote_from_string(consensus_text_md, + strlen(consensus_text_md), + NULL, NS_TYPE_CONSENSUS); tt_assert(con_md); tt_int_op(con_md->flavor,OP_EQ, FLAV_MICRODESC);