From b1ff8daeb521d1645bc35ffd7191599b7169c2bb Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 13 Dec 2012 17:34:05 -0500 Subject: Nuke uses of memcmp outside of unit tests We want to be saying fast_mem{cmp,eq,neq} when we're doing a comparison that's allowed to exit early, or tor_mem{cmp,eq,neq} when we need a data-invariant timing. Direct use of memcmp tends to imply that we haven't thought about the issue. --- src/common/aes.c | 3 ++- src/or/dirserv.c | 2 +- src/or/geoip.c | 7 ++++--- src/or/microdesc.c | 2 +- src/or/routerlist.c | 2 +- 5 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/common/aes.c b/src/common/aes.c index 2d64b8594..d8865d711 100644 --- a/src/common/aes.c +++ b/src/common/aes.c @@ -41,6 +41,7 @@ #include "aes.h" #include "util.h" #include "torlog.h" +#include "di_ops.h" #ifdef ANDROID /* Android's OpenSSL seems to have removed all of its Engine support. */ @@ -257,7 +258,7 @@ evaluate_ctr_for_aes(void) for (i=0; i<16; ++i) AES_ctr128_encrypt(&zero[i], &output[i], 1, &key, ivec, ivec_tmp, &pos); - if (memcmp(output, encrypt_zero, 16)) { + if (fast_memneq(output, encrypt_zero, 16)) { /* Counter mode is buggy */ log_notice(LD_CRYPTO, "This OpenSSL has a buggy version of counter mode; " "not using it."); diff --git a/src/or/dirserv.c b/src/or/dirserv.c index 0eb1fb3c6..d080fe7b1 100644 --- a/src/or/dirserv.c +++ b/src/or/dirserv.c @@ -2269,7 +2269,7 @@ compare_routerinfo_by_ip_and_bw_(const void **a, const void **b) else if (first->addr > second->addr) return 1; - /* Potentially, this next bit could cause k n lg n memcmp calls. But in + /* Potentially, this next bit could cause k n lg n memeq calls. But in * reality, we will almost never get here, since addresses will usually be * different. */ diff --git a/src/or/geoip.c b/src/or/geoip.c index 2fd77d8b9..72a1983cb 100644 --- a/src/or/geoip.c +++ b/src/or/geoip.c @@ -224,7 +224,8 @@ static int geoip_ipv6_compare_entries_(const void **_a, const void **_b) { const geoip_ipv6_entry_t *a = *_a, *b = *_b; - return memcmp(a->ip_low.s6_addr, b->ip_low.s6_addr, sizeof(struct in6_addr)); + return fast_memcmp(a->ip_low.s6_addr, b->ip_low.s6_addr, + sizeof(struct in6_addr)); } /** bsearch helper: return -1, 1, or 0 based on comparison of an IPv6 @@ -235,10 +236,10 @@ geoip_ipv6_compare_key_to_entry_(const void *_key, const void **_member) const struct in6_addr *addr = (struct in6_addr *)_key; const geoip_ipv6_entry_t *entry = *_member; - if (memcmp(addr->s6_addr, entry->ip_low.s6_addr, + if (fast_memcmp(addr->s6_addr, entry->ip_low.s6_addr, sizeof(struct in6_addr)) < 0) return -1; - else if (memcmp(addr->s6_addr, entry->ip_high.s6_addr, + else if (fast_memcmp(addr->s6_addr, entry->ip_high.s6_addr, sizeof(struct in6_addr)) > 0) return 1; else diff --git a/src/or/microdesc.c b/src/or/microdesc.c index 7602a9345..788a7b1e1 100644 --- a/src/or/microdesc.c +++ b/src/or/microdesc.c @@ -479,7 +479,7 @@ microdesc_cache_rebuild(microdesc_cache_t *cache, int force) if (PREDICT_UNLIKELY( md->bodylen < 9 || fast_memneq(md->body, "onion-key", 9) != 0)) { /* XXXX once bug 2022 is solved, we can kill this block and turn it - * into just the tor_assert(!memcmp) */ + * into just the tor_assert(fast_memeq) */ off_t avail = cache->cache_content->size - md->off; char *bad_str; tor_assert(avail >= 0); diff --git a/src/or/routerlist.c b/src/or/routerlist.c index 5536d1c61..6fff70b6b 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -4436,7 +4436,7 @@ update_consensus_router_descriptor_downloads(time_t now, int is_vote, sd->signed_descriptor_digest, DIGEST_LEN)) { /* We have a descriptor with this digest, but either there is no * entry in routerlist with the same ID (!ri), or there is one, - * but the identity digest differs (memcmp). + * but the identity digest differs (memneq). */ smartlist_add(no_longer_old, sd); ++n_in_oldrouters; /* We have it in old_routers. */ -- cgit v1.2.3