GitLab

A malicious/misbehaving set of directory authorities can cause a client to fail an assertion if they create a consensus that swaps descriptor digests between router entries and a client already has the descriptors for those routers.

in update_consensus_router_descriptor_downloads()

  SMARTLIST_FOREACH_BEGIN(consensus->routerstatus_list, void *, rsp) {
      routerstatus_t *rs =
        is_vote ? &(((vote_routerstatus_t *)rsp)->status) : rsp;
      signed_descriptor_t *sd;
      if ((sd = router_get_by_descriptor_digest(rs->descriptor_digest))) {
        const routerinfo_t *ri;
        ++n_have;
        if (!(ri = router_get_by_id_digest(rs->identity_digest)) ||
            tor_memneq(ri->cache_info.signed_descriptor_digest,
                   sd->signed_descriptor_digest, DIGEST_LEN)) {

If rs && sd && ri && the descriptor digests are not equal, then

in routerlist_remove_old()

  tor_assert(0 <= idx && idx < smartlist_len(rl->old_routers));
  /* XXXX edmanm's bridge relay triggered the following assert while
   * running 0.2.0.12-alpha.  If anybody triggers this again, see if we
   * can get a backtrace. */
  tor_assert(smartlist_get(rl->old_routers, idx) == sd);

Both assertions are triggerable because sd is assumed to be in old_routers. If the consensus specifies a valid but wrong descriptor digest for a router (i.e. they swap two of them), then the client will compare that new digest to the one it already has in the routerinfo. They will be different, so the client will assume it already has a new descriptor and that it previously moved the old descriptor into rl->old_routers (despite the fact that clients don't cache them and old_routers is empty). When we try to retrieve the cached descriptor, we assert.

If we're a relay, then we probably have descriptors in old_routers but not the one we're looking for. Therefore, these assumption are false, because we're comparing two different routers, thus resulting in the crash.

Brought to you by your friendly neighborhood cat.