commit c8a0cceae279eb2b27c51a358e00942a9967ca77 Author: Nick Mathewson <nickm@torproject.org> Date: Mon May 7 12:38:28 2012 -0400 Check more thoroughly for dups when parsing networkstatus parameters See changes file for details. Partial fix for bug 5786; fix on 0.2.2.2-alpha. --- changes/bug5786_nodups | 7 +++++++ src/or/dirvote.c | 4 ++-- src/or/routerparse.c | 17 +++++++++++++++++ 3 files changed, 26 insertions(+), 2 deletions(-) diff --git a/changes/bug5786_nodups b/changes/bug5786_nodups new file mode 100644 index 0000000..795b668 --- /dev/null +++ b/changes/bug5786_nodups @@ -0,0 +1,7 @@ + o Major bugfixes (directory authority): + - Check more thoroughly to prevent a rogue authority from + double-voting on any consensus directory parameter. Previously, + authorities would crash in this case if the total number of votes + for any parameter exceeded the number of active voters, but would + let it pass otherwise. Partial fix for bug 5786; bugfix on + 0.2.2.2-alpha. diff --git a/src/or/dirvote.c b/src/or/dirvote.c index cddb658..4848917 100644 --- a/src/or/dirvote.c +++ b/src/or/dirvote.c @@ -668,10 +668,10 @@ dirvote_compute_params(smartlist_t *votes, int method, int total_authorities) const char *next_param; int ok=0; eq = strchr(param, '='); - tor_assert(i<n_votes); + tor_assert(i<n_votes); /* Make sure we prevented vote-stuffing. */ vals[i++] = (int32_t) tor_parse_long(eq+1, 10, INT32_MIN, INT32_MAX, &ok, NULL); - tor_assert(ok); + tor_assert(ok); /* Already checked these when parsing. */ if (param_sl_idx+1 == smartlist_len(param_list)) next_param = NULL; diff --git a/src/or/routerparse.c b/src/or/routerparse.c index 7cc8b96..c5ce174 100644 --- a/src/or/routerparse.c +++ b/src/or/routerparse.c @@ -2821,6 +2821,7 @@ networkstatus_parse_vote_from_string(const char *s, const char **eos_out, int i, inorder, n_signatures = 0; memarea_t *area = NULL, *rs_area = NULL; consensus_flavor_t flav = FLAV_NS; + char *last_kwd=NULL; tor_assert(s); @@ -2977,15 +2978,18 @@ networkstatus_parse_vote_from_string(const char *s, const char **eos_out, tok = find_opt_by_keyword(tokens, K_PARAMS); if (tok) { + int any_dups = 0; inorder = 1; ns->net_params = smartlist_new(); for (i = 0; i < tok->n_args; ++i) { int ok=0; char *eq = strchr(tok->args[i], '='); + size_t eq_pos; if (!eq) { log_warn(LD_DIR, "Bad element '%s' in params", escaped(tok->args[i])); goto err; } + eq_pos = eq-tok->args[i]; tor_parse_long(eq+1, 10, INT32_MIN, INT32_MAX, &ok, NULL); if (!ok) { log_warn(LD_DIR, "Bad element '%s' in params", escaped(tok->args[i])); @@ -2995,12 +2999,24 @@ networkstatus_parse_vote_from_string(const char *s, const char **eos_out, log_warn(LD_DIR, "%s >= %s", tok->args[i-1], tok->args[i]); inorder = 0; } + if (last_kwd && eq_pos == strlen(last_kwd) && + fast_memeq(last_kwd, tok->args[i], eq_pos)) { + log_warn(LD_DIR, "Duplicate value for %s parameter", + escaped(tok->args[i])); + any_dups = 1; + } + tor_free(last_kwd); + last_kwd = tor_strndup(tok->args[i], eq_pos); smartlist_add(ns->net_params, tor_strdup(tok->args[i])); } if (!inorder) { log_warn(LD_DIR, "params not in order"); goto err; } + if (any_dups) { + log_warn(LD_DIR, "Duplicate in parameters"); + goto err; + } } ns->voters = smartlist_new(); @@ -3339,6 +3355,7 @@ networkstatus_parse_vote_from_string(const char *s, const char **eos_out, } if (rs_area) memarea_drop_all(rs_area); + tor_free(last_kwd); return ns; }