Commit bf3e32a4 authored by John Brooks's avatar John Brooks
Browse files

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.
parent 61c0bae4

changes/bug19032

0 → 100644
+4 −0
Original line number Original line Diff line number Diff line 
  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.

src/or/dirserv.c

+16 −15
Original line number Original line Diff line number Diff line
@@ -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. */

 * Also, set the is_exit flag of each router appropriately. */

static void

static void

dirserv_compute_performance_thresholds(const smartlist_t *routers,

dirserv_compute_performance_thresholds(digestmap_t *omit_as_sybil)

                                       digestmap_t *omit_as_sybil)

{

{

  int n_active, n_active_nonexit, n_familiar;

  int n_active, n_active_nonexit, n_familiar;

  uint32_t *uptimes, *bandwidths_kb, *bandwidths_excluding_exits_kb;

  uint32_t *uptimes, *bandwidths_kb, *bandwidths_excluding_exits_kb;

  long *tks;

  long *tks;

  double *mtbfs, *wfus;

  double *mtbfs, *wfus;

  smartlist_t *nodelist;

  time_t now = time(NULL);

  time_t now = time(NULL);

  const or_options_t *options = get_options();

  const or_options_t *options = get_options();
@@ -1446,27 +1446,28 @@ dirserv_compute_performance_thresholds(const smartlist_t *routers, 
  guard_tk = 0;

  guard_tk = 0;

  guard_wfu = 0;

  guard_wfu = 0;





  nodelist_assert_ok();

  nodelist = nodelist_get_list();



  /* Initialize arrays that will hold values for each router.  We'll

  /* Initialize arrays that will hold values for each router.  We'll

   * sort them and use that to compute thresholds. */

   * sort them and use that to compute thresholds. */

  n_active = n_active_nonexit = 0;

  n_active = n_active_nonexit = 0;

  /* Uptime for every active router. */

  /* 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. */

  /* 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. */

  /* Bandwidth for every active non-exit router. */

  bandwidths_excluding_exits_kb =

  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. */

  /* 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. */

  /* 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. */

  /* Weighted fractional uptime for each active router. */

  wfus = tor_calloc(smartlist_len(routers), sizeof(double));

  wfus = tor_calloc(smartlist_len(nodelist), sizeof(double));



  nodelist_assert_ok();





  /* Now, fill in the arrays. */

  /* Now, fill in the arrays. */

  SMARTLIST_FOREACH_BEGIN(nodelist_get_list(), node_t *, node) {

  SMARTLIST_FOREACH_BEGIN(nodelist, node_t *, node) {

    if (options->BridgeAuthoritativeDir &&

    if (options->BridgeAuthoritativeDir &&

        node->ri &&

        node->ri &&

        node->ri->purpose != ROUTER_PURPOSE_BRIDGE)

        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. */

   * fill wfus with the wfu of every such "familiar" router. */

  n_familiar = 0;

  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,

      if (router_counts_toward_thresholds(node, now,

                                          omit_as_sybil, require_mbw)) {

                                          omit_as_sybil, require_mbw)) {

        routerinfo_t *ri = node->ri;

        routerinfo_t *ri = node->ri;
@@ -1596,11 +1597,11 @@ dirserv_compute_performance_thresholds(const smartlist_t *routers, 
 * networkstatus_getinfo_by_purpose().

 * networkstatus_getinfo_by_purpose().

 */

 */

void

void

dirserv_compute_bridge_flag_thresholds(const smartlist_t *routers)

dirserv_compute_bridge_flag_thresholds()

{

{





  digestmap_t *omit_as_sybil = digestmap_new();

  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);

  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() */

   * this must come before dirserv_compute_performance_thresholds() */

  dirserv_count_measured_bws(routers);

  dirserv_count_measured_bws(routers);





  dirserv_compute_performance_thresholds(routers, omit_as_sybil);

  dirserv_compute_performance_thresholds(omit_as_sybil);





  routerstatuses = smartlist_new();

  routerstatuses = smartlist_new();

  microdescriptors = smartlist_new();

  microdescriptors = smartlist_new();

src/or/dirserv.h

+1 −1
Original line number Original line Diff line number Diff line
@@ -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,

int dirserv_dump_directory_to_string(char **dir_out,

                                     crypto_pk_t *private_key);

                                     crypto_pk_t *private_key);

char *dirserv_get_flag_thresholds_line(void);

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_from_authorities(const or_options_t *options);

int directory_fetches_dir_info_early(const or_options_t *options);

int directory_fetches_dir_info_early(const or_options_t *options);

src/or/networkstatus.c

+1 −2
Original line number Original line Diff line number Diff line
@@ -1697,11 +1697,10 @@ networkstatus_dump_bridge_status_to_file(time_t now) 
  char *fname = NULL;

  char *fname = NULL;

  char *thresholds = NULL;

  char *thresholds = NULL;

  char *published_thresholds_and_status = NULL;

  char *published_thresholds_and_status = NULL;

  routerlist_t *rl = router_get_routerlist();

  char published[ISO_TIME_LEN+1];

  char published[ISO_TIME_LEN+1];





  format_iso_time(published, now);

  format_iso_time(published, now);

  dirserv_compute_bridge_flag_thresholds(rl->routers);

  dirserv_compute_bridge_flag_thresholds();

  thresholds = dirserv_get_flag_thresholds_line();

  thresholds = dirserv_get_flag_thresholds_line();

  tor_asprintf(&published_thresholds_and_status,

  tor_asprintf(&published_thresholds_and_status,

               "published %s\nflag-thresholds %s\n%s",

               "published %s\nflag-thresholds %s\n%s",