[tor-commits] [tor/release-0.2.7] Fix out-of-bounds write during voting with duplicate ed25519 keys

nickm at torproject.org nickm at torproject.org
Tue Jul 5 16:25:46 UTC 2016


commit bf3e32a45288b64e5535e02f40bd2bcb93c8a520
Author: John Brooks <special at torproject.org>
Date:   Wed May 11 12:13:22 2016 -0400

    Fix out-of-bounds write during voting with duplicate ed25519 keys
    
    In dirserv_compute_performance_thresholds, we allocate arrays based
    on the length of 'routers', a list of routerinfo_t, but loop over
    the nodelist. The 'routers' list may be shorter when relays were
    filtered by routers_make_ed_keys_unique, leading to an out-of-bounds
    write on directory authorities.
    
    This bug was originally introduced in 26e89742, but it doesn't look
    possible to trigger until routers_make_ed_keys_unique was introduced
    in 13a31e72.
    
    Fixes bug 19032; bugfix on tor 0.2.8.2-alpha.
---
 changes/bug19032       |  4 ++++
 src/or/dirserv.c       | 31 ++++++++++++++++---------------
 src/or/dirserv.h       |  2 +-
 src/or/networkstatus.c |  3 +--
 4 files changed, 22 insertions(+), 18 deletions(-)

diff --git a/changes/bug19032 b/changes/bug19032
new file mode 100644
index 0000000..93f17c2
--- /dev/null
+++ b/changes/bug19032
@@ -0,0 +1,4 @@
+  o Major bugfixes (security, directory authorities):
+    - Fix a crash and out-of-bounds write during authority voting, when the
+      list of relays includes duplicate ed25519 identity keys. Fixes bug 19032;
+      bugfix on 0.2.8.2-alpha.
diff --git a/src/or/dirserv.c b/src/or/dirserv.c
index ae67e8e..a367669 100644
--- a/src/or/dirserv.c
+++ b/src/or/dirserv.c
@@ -1422,13 +1422,13 @@ router_counts_toward_thresholds(const node_t *node, time_t now,
  *
  * Also, set the is_exit flag of each router appropriately. */
 static void
-dirserv_compute_performance_thresholds(const smartlist_t *routers,
-                                       digestmap_t *omit_as_sybil)
+dirserv_compute_performance_thresholds(digestmap_t *omit_as_sybil)
 {
   int n_active, n_active_nonexit, n_familiar;
   uint32_t *uptimes, *bandwidths_kb, *bandwidths_excluding_exits_kb;
   long *tks;
   double *mtbfs, *wfus;
+  smartlist_t *nodelist;
   time_t now = time(NULL);
   const or_options_t *options = get_options();
 
@@ -1446,27 +1446,28 @@ dirserv_compute_performance_thresholds(const smartlist_t *routers,
   guard_tk = 0;
   guard_wfu = 0;
 
+  nodelist_assert_ok();
+  nodelist = nodelist_get_list();
+
   /* Initialize arrays that will hold values for each router.  We'll
    * sort them and use that to compute thresholds. */
   n_active = n_active_nonexit = 0;
   /* Uptime for every active router. */
-  uptimes = tor_calloc(smartlist_len(routers), sizeof(uint32_t));
+  uptimes = tor_calloc(smartlist_len(nodelist), sizeof(uint32_t));
   /* Bandwidth for every active router. */
-  bandwidths_kb = tor_calloc(smartlist_len(routers), sizeof(uint32_t));
+  bandwidths_kb = tor_calloc(smartlist_len(nodelist), sizeof(uint32_t));
   /* Bandwidth for every active non-exit router. */
   bandwidths_excluding_exits_kb =
-    tor_calloc(smartlist_len(routers), sizeof(uint32_t));
+    tor_calloc(smartlist_len(nodelist), sizeof(uint32_t));
   /* Weighted mean time between failure for each active router. */
-  mtbfs = tor_calloc(smartlist_len(routers), sizeof(double));
+  mtbfs = tor_calloc(smartlist_len(nodelist), sizeof(double));
   /* Time-known for each active router. */
-  tks = tor_calloc(smartlist_len(routers), sizeof(long));
+  tks = tor_calloc(smartlist_len(nodelist), sizeof(long));
   /* Weighted fractional uptime for each active router. */
-  wfus = tor_calloc(smartlist_len(routers), sizeof(double));
-
-  nodelist_assert_ok();
+  wfus = tor_calloc(smartlist_len(nodelist), sizeof(double));
 
   /* Now, fill in the arrays. */
-  SMARTLIST_FOREACH_BEGIN(nodelist_get_list(), node_t *, node) {
+  SMARTLIST_FOREACH_BEGIN(nodelist, node_t *, node) {
     if (options->BridgeAuthoritativeDir &&
         node->ri &&
         node->ri->purpose != ROUTER_PURPOSE_BRIDGE)
@@ -1542,7 +1543,7 @@ dirserv_compute_performance_thresholds(const smartlist_t *routers,
    * fill wfus with the wfu of every such "familiar" router. */
   n_familiar = 0;
 
-  SMARTLIST_FOREACH_BEGIN(nodelist_get_list(), node_t *, node) {
+  SMARTLIST_FOREACH_BEGIN(nodelist, node_t *, node) {
       if (router_counts_toward_thresholds(node, now,
                                           omit_as_sybil, require_mbw)) {
         routerinfo_t *ri = node->ri;
@@ -1596,11 +1597,11 @@ dirserv_compute_performance_thresholds(const smartlist_t *routers,
  * networkstatus_getinfo_by_purpose().
  */
 void
-dirserv_compute_bridge_flag_thresholds(const smartlist_t *routers)
+dirserv_compute_bridge_flag_thresholds()
 {
 
   digestmap_t *omit_as_sybil = digestmap_new();
-  dirserv_compute_performance_thresholds(routers, omit_as_sybil);
+  dirserv_compute_performance_thresholds(omit_as_sybil);
   digestmap_free(omit_as_sybil, NULL);
 }
 
@@ -2870,7 +2871,7 @@ dirserv_generate_networkstatus_vote_obj(crypto_pk_t *private_key,
    * this must come before dirserv_compute_performance_thresholds() */
   dirserv_count_measured_bws(routers);
 
-  dirserv_compute_performance_thresholds(routers, omit_as_sybil);
+  dirserv_compute_performance_thresholds(omit_as_sybil);
 
   routerstatuses = smartlist_new();
   microdescriptors = smartlist_new();
diff --git a/src/or/dirserv.h b/src/or/dirserv.h
index b16a67c..4bb3072 100644
--- a/src/or/dirserv.h
+++ b/src/or/dirserv.h
@@ -50,7 +50,7 @@ int list_server_status_v1(smartlist_t *routers, char **router_status_out,
 int dirserv_dump_directory_to_string(char **dir_out,
                                      crypto_pk_t *private_key);
 char *dirserv_get_flag_thresholds_line(void);
-void dirserv_compute_bridge_flag_thresholds(const smartlist_t *routers);
+void dirserv_compute_bridge_flag_thresholds(void);
 
 int directory_fetches_from_authorities(const or_options_t *options);
 int directory_fetches_dir_info_early(const or_options_t *options);
diff --git a/src/or/networkstatus.c b/src/or/networkstatus.c
index a9b22ed..f72e9d5 100644
--- a/src/or/networkstatus.c
+++ b/src/or/networkstatus.c
@@ -1697,11 +1697,10 @@ networkstatus_dump_bridge_status_to_file(time_t now)
   char *fname = NULL;
   char *thresholds = NULL;
   char *published_thresholds_and_status = NULL;
-  routerlist_t *rl = router_get_routerlist();
   char published[ISO_TIME_LEN+1];
 
   format_iso_time(published, now);
-  dirserv_compute_bridge_flag_thresholds(rl->routers);
+  dirserv_compute_bridge_flag_thresholds();
   thresholds = dirserv_get_flag_thresholds_line();
   tor_asprintf(&published_thresholds_and_status,
                "published %s\nflag-thresholds %s\n%s",





More information about the tor-commits mailing list