Browse Source

Prevent int underflow in dirvote.c compare_vote_rs_.

This should be "impossible" without making a SHA1 collision, but
let's not keep the assumption that SHA1 collisions are super-hard.

This prevents another case related to 21278.  There should be no
behavioral change unless -ftrapv is on.
Nick Mathewson 7 years ago
parent
commit
a0ef3cf088
1 changed files with 22 additions and 8 deletions
  1. 22 8
      src/or/dirvote.c

+ 22 - 8
src/or/dirvote.c

@@ -421,16 +421,30 @@ compare_vote_rs(const vote_routerstatus_t *a, const vote_routerstatus_t *b)
                        b->status.descriptor_digest,
                        DIGEST_LEN)))
     return r;
-  if ((r = (int)(b->status.published_on - a->status.published_on)))
-    return r;
+  /* If we actually reached this point, then the identities and
+   * the descriptor digests matched, so somebody is making SHA1 collisions.
+   */
+#define CMP_FIELD(utype, itype, field) do {                             \
+    utype aval = (utype) (itype) a->status.field;                       \
+    utype bval = (utype) (itype) b->status.field;                       \
+    utype u = bval - aval;                                              \
+    itype r2 = (itype) u;                                               \
+    if (r2 < 0) {                                                       \
+      return -1;                                                        \
+    } else if (r2 > 0) {                                                \
+      return 1;                                                         \
+    }                                                                   \
+  } while (0)
+
+  CMP_FIELD(uint64_t, int64_t, published_on);
+
   if ((r = strcmp(b->status.nickname, a->status.nickname)))
     return r;
-  if ((r = (((int)b->status.addr) - ((int)a->status.addr))))
-    return r;
-  if ((r = (((int)b->status.or_port) - ((int)a->status.or_port))))
-    return r;
-  if ((r = (((int)b->status.dir_port) - ((int)a->status.dir_port))))
-    return r;
+
+  CMP_FIELD(unsigned, int, addr);
+  CMP_FIELD(unsigned, int, or_port);
+  CMP_FIELD(unsigned, int, dir_port);
+
   return 0;
 }