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.
This commit is contained in:
Nick Mathewson 2018-09-11 10:32:17 -04:00
parent e014b72b73
commit abaca3fc8c
9 changed files with 85 additions and 47 deletions

View File

@ -401,7 +401,8 @@ format_networkstatus_vote(crypto_pk_t *private_signing_key,
{ {
networkstatus_t *v; 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))) { v3_ns->type))) {
log_err(LD_BUG,"Generated a networkstatus %s we couldn't parse: " log_err(LD_BUG,"Generated a networkstatus %s we couldn't parse: "
"<<%s>>", "<<%s>>",
@ -2398,7 +2399,8 @@ networkstatus_compute_consensus(smartlist_t *votes,
{ {
networkstatus_t *c; 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))) { NS_TYPE_CONSENSUS))) {
log_err(LD_BUG, "Generated a networkstatus consensus we couldn't " log_err(LD_BUG, "Generated a networkstatus consensus we couldn't "
"parse."); "parse.");
@ -3121,7 +3123,8 @@ dirvote_add_vote(const char *vote_body, const char **msg_out, int *status_out)
*msg_out = NULL; *msg_out = NULL;
again: 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); NS_TYPE_VOTE);
if (!end_of_vote) if (!end_of_vote)
end_of_vote = vote_body + strlen(vote_body); end_of_vote = vote_body + strlen(vote_body);
@ -3379,7 +3382,9 @@ dirvote_compute_consensuses(void)
flavor_name); flavor_name);
continue; 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); NS_TYPE_CONSENSUS);
if (!consensus) { if (!consensus) {
log_warn(LD_DIR, "Couldn't parse %s consensus we generated!", 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. */ * just in case we break detached signature processing at some point. */
{ {
networkstatus_t *v = networkstatus_parse_vote_from_string( networkstatus_t *v = networkstatus_parse_vote_from_string(
pc->body, NULL, pc->body, strlen(pc->body), NULL,
NS_TYPE_CONSENSUS); NS_TYPE_CONSENSUS);
tor_assert(v); tor_assert(v);
networkstatus_vote_free(v); networkstatus_vote_free(v);

View File

@ -1861,7 +1861,9 @@ networkstatus_set_current_consensus(const char *consensus,
} }
/* Make sure it's parseable. */ /* 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) { if (!c) {
log_warn(LD_DIR, "Unable to parse networkstatus consensus"); log_warn(LD_DIR, "Unable to parse networkstatus consensus");
result = -2; result = -2;

View File

@ -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 /** Set <b>digests</b> to all the digests of the consensus document in
* <b>s</b> */ * <b>s</b> */
int 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", "network-status-version",
"\ndirectory-signature", "\ndirectory-signature",
' '); ' ');
@ -2489,18 +2490,19 @@ authority_cert_parse_from_string(const char *s, const char **end_of_string)
return NULL; 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, * 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. * return the start of the directory footer, or the next directory signature.
* If none is found, return the end of the string. */ * If none is found, return the end of the string. */
static inline const char * 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; const char *eos, *footer, *sig;
if ((eos = strstr(s, "\nr "))) if ((eos = tor_memstr(s, s_eos - s, "\nr ")))
++eos; ++eos;
else else
eos = s + strlen(s); eos = s_eos;
footer = tor_memstr(s, eos-s, "\ndirectory-footer"); footer = tor_memstr(s, eos-s, "\ndirectory-footer");
sig = tor_memstr(s, eos-s, "\ndirectory-signature"); sig = tor_memstr(s, eos-s, "\ndirectory-signature");
@ -2632,7 +2634,8 @@ summarize_protover_flags(protover_summary_flags_t *out,
**/ **/
STATIC routerstatus_t * STATIC routerstatus_t *
routerstatus_parse_entry_from_string(memarea_t *area, 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, networkstatus_t *vote,
vote_routerstatus_t *vote_rs, vote_routerstatus_t *vote_rs,
int consensus_method, int consensus_method,
@ -2651,7 +2654,7 @@ routerstatus_parse_entry_from_string(memarea_t *area,
flav = FLAV_NS; flav = FLAV_NS;
tor_assert(flav == FLAV_NS || flav == FLAV_MICRODESC); 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)) { if (tokenize_string(area,*s, eos, tokens, rtrstatus_token_table,0)) {
log_warn(LD_DIR, "Error tokenizing router status"); 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 /** Parse a v3 networkstatus vote, opinion, or consensus (depending on
* ns_type), from <b>s</b>, and return the result. Return NULL on failure. */ * ns_type), from <b>s</b>, and return the result. Return NULL on failure. */
networkstatus_t * 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) networkstatus_type_t ns_type)
{ {
smartlist_t *tokens = smartlist_new(); 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; memarea_t *area = NULL, *rs_area = NULL;
consensus_flavor_t flav = FLAV_NS; consensus_flavor_t flav = FLAV_NS;
char *last_kwd=NULL; char *last_kwd=NULL;
const char *eos = s + s_len;
tor_assert(s); tor_assert(s);
if (eos_out) if (eos_out)
*eos_out = NULL; *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, 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"); log_warn(LD_DIR, "Unable to compute digest of network-status");
goto err; goto err;
} }
area = memarea_new(); 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, if (tokenize_string(area, s, end_of_header, tokens,
(ns_type == NS_TYPE_CONSENSUS) ? (ns_type == NS_TYPE_CONSENSUS) ?
networkstatus_consensus_token_table : 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) { if (ns_type != NS_TYPE_CONSENSUS) {
const char *end_of_cert = NULL; 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; goto err;
++cert; ++cert;
ns->cert = authority_cert_parse_from_string(cert, &end_of_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; s = end_of_header;
ns->routerstatus_list = smartlist_new(); ns->routerstatus_list = smartlist_new();
while (!strcmpstart(s, "r ")) { while (eos-s >= 2 && fast_memeq(s, "r ", 2)) {
if (ns->type != NS_TYPE_CONSENSUS) { if (ns->type != NS_TYPE_CONSENSUS) {
vote_routerstatus_t *rs = tor_malloc_zero(sizeof(vote_routerstatus_t)); 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)) { rs, 0, 0)) {
smartlist_add(ns->routerstatus_list, rs); smartlist_add(ns->routerstatus_list, rs);
} else { } else {
@ -3779,7 +3786,8 @@ networkstatus_parse_vote_from_string(const char *s, const char **eos_out,
} }
} else { } else {
routerstatus_t *rs; 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, NULL, NULL,
ns->consensus_method, ns->consensus_method,
flav))) { flav))) {
@ -3824,10 +3832,10 @@ networkstatus_parse_vote_from_string(const char *s, const char **eos_out,
/* Parse footer; check signature. */ /* Parse footer; check signature. */
footer_tokens = smartlist_new(); 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; ++end_of_footer;
else else
end_of_footer = s + strlen(s); end_of_footer = eos;
if (tokenize_string(area,s, end_of_footer, footer_tokens, if (tokenize_string(area,s, end_of_footer, footer_tokens,
networkstatus_vote_footer_token_table, 0)) { networkstatus_vote_footer_token_table, 0)) {
log_warn(LD_DIR, "Error tokenizing network-status vote footer."); log_warn(LD_DIR, "Error tokenizing network-status vote footer.");

View File

@ -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_router_hash(const char *s, size_t s_len, char *digest);
int router_get_dir_hash(const char *s, 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); common_digests_t *digests);
int router_get_networkstatus_v3_signed_boundaries(const char *s, size_t len, int router_get_networkstatus_v3_signed_boundaries(const char *s, size_t len,
const char **start_out, 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 compare_vote_routerstatus_entries(const void **_a, const void **_b);
int networkstatus_verify_bw_weights(networkstatus_t *ns, int); int networkstatus_verify_bw_weights(networkstatus_t *ns, int);
networkstatus_t *networkstatus_parse_vote_from_string(const char *s, networkstatus_t *networkstatus_parse_vote_from_string(const char *s,
size_t len,
const char **eos_out, const char **eos_out,
enum networkstatus_type_t ns_type); enum networkstatus_type_t ns_type);
ns_detached_signatures_t *networkstatus_parse_detached_signatures( ns_detached_signatures_t *networkstatus_parse_detached_signatures(
@ -139,7 +140,8 @@ STATIC void dump_desc_fifo_cleanup(void);
struct memarea_t; struct memarea_t;
STATIC routerstatus_t *routerstatus_parse_entry_from_string( STATIC routerstatus_t *routerstatus_parse_entry_from_string(
struct memarea_t *area, struct memarea_t *area,
const char **s, smartlist_t *tokens, const char **s, const char *eos,
smartlist_t *tokens,
networkstatus_t *vote, networkstatus_t *vote,
vote_routerstatus_t *vote_rs, vote_routerstatus_t *vote_rs,
int consensus_method, int consensus_method,

View File

@ -59,13 +59,13 @@ int
fuzz_main(const uint8_t *data, size_t sz) fuzz_main(const uint8_t *data, size_t sz)
{ {
networkstatus_t *ns; networkstatus_t *ns;
char *str = tor_memdup_nulterm(data, sz);
const char *eos = NULL; const char *eos = NULL;
networkstatus_type_t tp = NS_TYPE_CONSENSUS; networkstatus_type_t tp = NS_TYPE_CONSENSUS;
if (tor_memstr(data, MIN(sz, 1024), "tus vote")) if (tor_memstr(data, MIN(sz, 1024), "tus vote"))
tp = NS_TYPE_VOTE; tp = NS_TYPE_VOTE;
const char *what = (tp == NS_TYPE_CONSENSUS) ? "consensus" : "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, &eos,
tp); tp);
if (ns) { if (ns) {
@ -74,6 +74,6 @@ fuzz_main(const uint8_t *data, size_t sz)
} else { } else {
log_debug(LD_GENERAL, "Parsing as %s failed", what); log_debug(LD_GENERAL, "Parsing as %s failed", what);
} }
tor_free(str);
return 0; return 0;
} }

View File

@ -52,24 +52,24 @@ fuzz_cleanup(void)
int int
fuzz_main(const uint8_t *data, size_t sz) fuzz_main(const uint8_t *data, size_t sz)
{ {
char *str = tor_memdup_nulterm(data, sz);
const char *s; const char *s;
routerstatus_t *rs_ns = NULL, *rs_md = NULL, *rs_vote = NULL; routerstatus_t *rs_ns = NULL, *rs_md = NULL, *rs_vote = NULL;
vote_routerstatus_t *vrs = tor_malloc_zero(sizeof(*vrs)); vote_routerstatus_t *vrs = tor_malloc_zero(sizeof(*vrs));
smartlist_t *tokens = smartlist_new(); smartlist_t *tokens = smartlist_new();
const char *eos = (const char *)data + sz;
s = str; s = (const char *)data;
rs_ns = routerstatus_parse_entry_from_string(area, &s, tokens, rs_ns = routerstatus_parse_entry_from_string(area, &s, eos, tokens,
NULL, NULL, 26, FLAV_NS); NULL, NULL, 26, FLAV_NS);
tor_assert(smartlist_len(tokens) == 0); tor_assert(smartlist_len(tokens) == 0);
s = str; s = (const char *)data;
rs_md = routerstatus_parse_entry_from_string(area, &s, tokens, rs_md = routerstatus_parse_entry_from_string(area, &s, eos, tokens,
NULL, NULL, 26, FLAV_MICRODESC); NULL, NULL, 26, FLAV_MICRODESC);
tor_assert(smartlist_len(tokens) == 0); tor_assert(smartlist_len(tokens) == 0);
s = str; s = (const char *)data;
rs_vote = routerstatus_parse_entry_from_string(area, &s, tokens, rs_vote = routerstatus_parse_entry_from_string(area, &s, eos, tokens,
dummy_vote, vrs, 26, FLAV_NS); dummy_vote, vrs, 26, FLAV_NS);
tor_assert(smartlist_len(tokens) == 0); tor_assert(smartlist_len(tokens) == 0);
@ -81,6 +81,6 @@ fuzz_main(const uint8_t *data, size_t sz)
vote_routerstatus_free(vrs); vote_routerstatus_free(vrs);
memarea_clear(area); memarea_clear(area);
smartlist_free(tokens); smartlist_free(tokens);
tor_free(str);
return 0; return 0;
} }

View File

@ -70,6 +70,23 @@
#define NS_MODULE dir #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 static void
test_dir_nicknames(void *arg) test_dir_nicknames(void *arg)
{ {
@ -2888,7 +2905,7 @@ test_a_networkstatus(
sign_skey_leg1, sign_skey_leg1,
FLAV_NS); FLAV_NS);
tt_assert(consensus_text); 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); NS_TYPE_CONSENSUS);
tt_assert(con); tt_assert(con);
//log_notice(LD_GENERAL, "<<%s>>\n<<%s>>\n<<%s>>\n", //log_notice(LD_GENERAL, "<<%s>>\n<<%s>>\n<<%s>>\n",
@ -2900,7 +2917,7 @@ test_a_networkstatus(
sign_skey_leg1, sign_skey_leg1,
FLAV_MICRODESC); FLAV_MICRODESC);
tt_assert(consensus_text_md); 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); NS_TYPE_CONSENSUS);
tt_assert(con_md); tt_assert(con_md);
tt_int_op(con_md->flavor,OP_EQ, FLAV_MICRODESC); tt_int_op(con_md->flavor,OP_EQ, FLAV_MICRODESC);
@ -2999,13 +3016,13 @@ test_a_networkstatus(
tt_assert(consensus_text3); tt_assert(consensus_text3);
tt_assert(consensus_text_md2); tt_assert(consensus_text_md2);
tt_assert(consensus_text_md3); 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); NS_TYPE_CONSENSUS);
con3 = networkstatus_parse_vote_from_string(consensus_text3, NULL, con3 = networkstatus_parse_vote_from_string_(consensus_text3, NULL,
NS_TYPE_CONSENSUS); 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); 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); NS_TYPE_CONSENSUS);
tt_assert(con2); tt_assert(con2);
tt_assert(con3); tt_assert(con3);
@ -6020,9 +6037,10 @@ test_dir_assumed_flags(void *arg)
"192.168.0.1 9001 0\n" "192.168.0.1 9001 0\n"
"m thisoneislongerbecauseitisa256bitmddigest33\n" "m thisoneislongerbecauseitisa256bitmddigest33\n"
"s Fast Guard Stable\n"; "s Fast Guard Stable\n";
const char *eos = str1 + strlen(str1);
const char *cp = 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); 24, FLAV_MICRODESC);
tt_assert(rs); tt_assert(rs);
tt_assert(rs->is_flagged_running); tt_assert(rs->is_flagged_running);

View File

@ -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. */ /* dump the vote and try to parse it. */
v_text = format_networkstatus_vote(sign_skey, vote); v_text = format_networkstatus_vote(sign_skey, vote);
tt_assert(v_text); 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: done:
if (v_text) if (v_text)
@ -422,4 +424,3 @@ dir_common_construct_vote_3(networkstatus_t **vote, authority_cert_t *cert,
return 0; return 0;
} }

View File

@ -270,7 +270,9 @@ test_router_pick_directory_server_impl(void *arg)
construct_consensus(&consensus_text_md, now); construct_consensus(&consensus_text_md, now);
tt_assert(consensus_text_md); 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); NS_TYPE_CONSENSUS);
tt_assert(con_md); tt_assert(con_md);
tt_int_op(con_md->flavor,OP_EQ, FLAV_MICRODESC); tt_int_op(con_md->flavor,OP_EQ, FLAV_MICRODESC);