commit a0ef3cf0880e3cd343977b3fcbd0a2e7572f0cb4 Author: Nick Mathewson nickm@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; }