Merge branch 'bug5786'

4 files changed, 36 insertions, 5 deletions

diff --git a/changes/bug5786_nocrash b/changes/bug5786_nocrash
new file mode 100644
index 000000000..ec6c5d817
--- /dev/null
+++ b/changes/bug5786_nocrash
@@ -0,0 +1,7 @@
+  o Major bugfixes (directory authorties):
+    - When computing weight parameters, behave more robustly in the
+      presence of a bad bwweightscale value.  Previously, the
+      authorities would crash if they agreed on a sufficiently browken
+      weight_scale value: now, they use a reasonable default and carry
+      on. Partial fix for 5786; bugfix on 0.2.2.17-alpha.
+
diff --git a/changes/bug5786_nodups b/changes/bug5786_nodups
new file mode 100644
index 000000000..795b668a8
--- /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 cddb65824..20dc8c264 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;
@@ -1005,7 +1005,7 @@ networkstatus_compute_bw_weights_v10(smartlist_t *chunks, int64_t G,
   /* We cast down the weights to 32 bit ints on the assumption that
    * weight_scale is ~= 10000. We need to ensure a rogue authority
    * doesn't break this assumption to rig our weights */
-  tor_assert(0 < weight_scale && weight_scale < INT32_MAX);
+  tor_assert(0 < weight_scale && weight_scale <= INT32_MAX);
 
   /*
    * Provide Wgm=Wgg, Wmm=1, Wem=Wee, Weg=Wed. May later determine
@@ -1233,7 +1233,7 @@ networkstatus_compute_bw_weights_v9(smartlist_t *chunks, int64_t G, int64_t M,
   /* We cast down the weights to 32 bit ints on the assumption that
    * weight_scale is ~= 10000. We need to ensure a rogue authority
    * doesn't break this assumption to rig our weights */
-  tor_assert(0 < weight_scale && weight_scale < INT32_MAX);
+  tor_assert(0 < weight_scale && weight_scale <= INT32_MAX);
 
   if (Wgg < 0 || Wgg > weight_scale) {
     log_warn(LD_DIR, "Bw %s: Wgg="I64_FORMAT"! G="I64_FORMAT
@@ -2019,7 +2019,7 @@ networkstatus_compute_consensus(smartlist_t *votes,
       int ok=0;
       char *eq = strchr(bw_weight_param, '=');
       if (eq) {
-        weight_scale = tor_parse_long(eq+1, 10, INT32_MIN, INT32_MAX, &ok,
+        weight_scale = tor_parse_long(eq+1, 10, 1, INT32_MAX, &ok,
                                          NULL);
         if (!ok) {
           log_warn(LD_DIR, "Bad element '%s' in bw weight param",
diff --git a/src/or/routerparse.c b/src/or/routerparse.c
index 7cc8b965e..c5ce17491 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;
 }