[tor-commits] [tor/master] Prevent int underflow in dirvote.c compare_vote_rs_.

[nickm at torproject.org]

commit a0ef3cf0880e3cd343977b3fcbd0a2e7572f0cb4
Author: Nick Mathewson <nickm at torproject.org>
Date:   Mon Feb 13 14:07:35 2017 -0500

    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.
---
 src/or/dirvote.c | 30 ++++++++++++++++++++++--------
 1 file changed, 22 insertions(+), 8 deletions(-)

diff --git a/src/or/dirvote.c b/src/or/dirvote.c
index 2c10e78..738ab35 100644
--- a/src/or/dirvote.c
+++ b/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;
 }