[tor-commits] [tor/master] addr: Safeguard last resolved address index access

[dgoulet at torproject.org]

commit 5895aafe7ed86bded4c5a597113a616db7c0a44f
Author: David Goulet <dgoulet at torproject.org>
Date:   Wed Jun 24 08:27:24 2020 -0400

    addr: Safeguard last resolved address index access
    
    The last resolved address cache uses an index that is mapped to an address
    family (AF_INET and AF_INET6).
    
    This commit adds a conversion function from af to index and change the code to
    use that all the time only.
    
    In the process, this commit fixes a bug that the last resolved address
    accessors were using the af value insted of the index.
    
    Spotted by nickm during review
    
    Signed-off-by: David Goulet <dgoulet at torproject.org>
---
 src/app/config/resolve_addr.c | 47 +++++++++++++++++++++++++++----------------
 1 file changed, 30 insertions(+), 17 deletions(-)

diff --git a/src/app/config/resolve_addr.c b/src/app/config/resolve_addr.c
index 56548e6e7..de39f9df9 100644
--- a/src/app/config/resolve_addr.c
+++ b/src/app/config/resolve_addr.c
@@ -37,13 +37,30 @@
 
 /** Ease our life. Arrays containing state per address family. These are to
  * add semantic to the code so we know what is accessed. */
-#define IDX_IPV4 0 /* Index to AF_INET. */
-#define IDX_IPV6 1 /* Index to AF_INET6. */
-#define IDX_SIZE 2 /* How many indexes do we have. */
+#define IDX_NULL 0 /* Index to zeroed address object. */
+#define IDX_IPV4 1 /* Index to AF_INET. */
+#define IDX_IPV6 2 /* Index to AF_INET6. */
+#define IDX_SIZE 3 /* How many indexes do we have. */
 
 /** Last resolved addresses. */
 static tor_addr_t last_resolved_addrs[IDX_SIZE];
 
+static inline int
+af_to_idx(const int family)
+{
+  switch (family) {
+  case AF_INET:
+    return IDX_IPV4;
+  case AF_INET6:
+    return IDX_IPV6;
+  default:
+    /* It wouldn't be safe to just die here with an assert but we can heavily
+     * scream with a bug. Return the index of the NULL address. */
+    tor_assert_nonfatal_unreached();
+    return IDX_NULL;
+  }
+}
+
 /** Copy the last resolved address of family into addr_out.
  *
  * If not last resolved address existed, the addr_out is a null address (use
@@ -51,7 +68,7 @@ static tor_addr_t last_resolved_addrs[IDX_SIZE];
 void
 resolved_addr_get_last(int family, tor_addr_t *addr_out)
 {
-  tor_addr_copy(addr_out, &last_resolved_addrs[family]);
+  tor_addr_copy(addr_out, &last_resolved_addrs[af_to_idx(family)]);
 }
 
 /** Reset the last resolved address of family.
@@ -60,7 +77,7 @@ resolved_addr_get_last(int family, tor_addr_t *addr_out)
 void
 resolved_addr_reset_last(int family)
 {
-  tor_addr_make_null(&last_resolved_addrs[family], family);
+  tor_addr_make_null(&last_resolved_addrs[af_to_idx(family)], family);
 }
 
 /** @brief Return true iff the given IP address can be used as a valid
@@ -321,7 +338,7 @@ update_resolved_cache(const tor_addr_t *addr, const char *method_used,
                       const char *hostname_used)
 {
   /** Have we done a first resolve. This is used to control logging. */
-  static bool have_resolved_once[IDX_SIZE] = { false, false };
+  static bool have_resolved_once[IDX_SIZE] = { false, false, false };
   bool *done_one_resolve;
   bool have_hostname = false;
   tor_addr_t *last_resolved;
@@ -333,20 +350,16 @@ update_resolved_cache(const tor_addr_t *addr, const char *method_used,
   /* Do we have an hostname. */
   have_hostname = strlen(hostname_used) > 0;
 
-  switch (tor_addr_family(addr)) {
-  case AF_INET:
-    done_one_resolve = &have_resolved_once[IDX_IPV4];
-    last_resolved = &last_resolved_addrs[IDX_IPV4];
-    break;
-  case AF_INET6:
-    done_one_resolve = &have_resolved_once[IDX_IPV6];
-    last_resolved = &last_resolved_addrs[IDX_IPV6];
-    break;
-  default:
-    tor_assert_nonfatal_unreached();
+  int idx = af_to_idx(tor_addr_family(addr));
+  if (idx == IDX_NULL) {
+    /* Not suppose to happen and if it does, af_to_idx() screams loudly. */
     return;
   }
 
+  /* Get values from cache. */
+  done_one_resolve = &have_resolved_once[idx];
+  last_resolved = &last_resolved_addrs[idx];
+
   /* Same address last resolved. Ignore. */
   if (tor_addr_eq(last_resolved, addr)) {
     return;