Add checks and unit tests for get_interface_address* failure

Ensure that either a valid address is returned in address pointers,
or that the address data is zeroed on error.

Ensure that free_interface_address6_list handles NULL lists.

Add unit tests for get_interface_address* failure cases.

Fixes bug #17173.
Patch by fk/teor, not in any released version of tor.

changes/bug17173-socket-hack-rv

@@ -0,0 +1,10 @@
+  o Minor bug fixes (addresses, testing):
+    - Handle errors in get_interface_address6_via_udp_socket_hack by
+      returning an empty list (no addresses found). This bug was triggered
+      in ElectroBSD/FreeBSD jails.
+    - Ensure that either a valid address is returned in address pointers,
+      or that the address data is zeroed on error.
+    - Ensure that free_interface_address6_list handles NULL lists.
+    - Add unit tests for get_interface_address* failure cases.
+      Fixes bug #17173.
+      Patch by fk/teor, not in any released version of tor.

src/common/address.c

@@ -1506,8 +1506,8 @@ get_interface_addresses_ioctl(int severity)
  * Return a new smartlist of tor_addr_t on success, and NULL on failure.
  * (An empty smartlist indicates that we successfully learned that we have no
  * addresses.)  Log failure messages at <b>severity</b>. */
-STATIC smartlist_t *
-get_interface_addresses_raw(int severity)
+MOCK_IMPL(smartlist_t *,
+get_interface_addresses_raw,(int severity))
 {
   smartlist_t *result = NULL;
 #if defined(HAVE_IFADDRS_TO_SMARTLIST)
@@ -1547,10 +1547,10 @@ tor_addr_is_multicast(const tor_addr_t *a)
  * UDP socket trickery. Only look for address of given <b>family</b>.
  * Set result to *<b>addr</b>. Return 0 on success, -1 on failure.
  */
-STATIC int
-get_interface_address6_via_udp_socket_hack(int severity,
-                                           sa_family_t family,
-                                           tor_addr_t *addr)
+MOCK_IMPL(int,
+get_interface_address6_via_udp_socket_hack,(int severity,
+                                            sa_family_t family,
+                                            tor_addr_t *addr))
 {
   struct sockaddr_storage my_addr, target_addr;
   int sock=-1, r=-1;
@@ -1614,6 +1614,8 @@ get_interface_address6_via_udp_socket_hack(int severity,
  err:
   if (sock >= 0)
     tor_close_socket(sock);
+  if (r == -1)
+    memset(addr, 0, sizeof(tor_addr_t));
   return r;
 }
 
@@ -1632,6 +1634,8 @@ get_interface_address6,(int severity, sa_family_t family, tor_addr_t *addr))
   int rv = -1;
   tor_assert(addr);
 
+  memset(addr, 0, sizeof(tor_addr_t));
+
   /* Get a list of public or internal IPs in arbitrary order */
   addrs = get_interface_address6_list(severity, family, 1);
 
@@ -1656,8 +1660,10 @@ get_interface_address6,(int severity, sa_family_t family, tor_addr_t *addr))
 void
 free_interface_address6_list(smartlist_t *addrs)
 {
-  SMARTLIST_FOREACH(addrs, tor_addr_t *, a, tor_free(a));
-  smartlist_free(addrs);
+  if (addrs != NULL) {
+    SMARTLIST_FOREACH(addrs, tor_addr_t *, a, tor_free(a));
+    smartlist_free(addrs);
+  }
 }
 
 /** Return a smartlist of the IP addresses of type family from all interfaces
@@ -1975,6 +1981,8 @@ get_interface_address,(int severity, uint32_t *addr))
   tor_addr_t local_addr;
   int r;
 
+  memset(addr, 0, sizeof(uint32_t));
+
   r = get_interface_address6(severity, AF_INET, &local_addr);
   if (r>=0)
     *addr = tor_addr_to_ipv4h(&local_addr);

src/common/address.h

@@ -310,11 +310,11 @@ get_interface_address_list(int severity, int include_internal)
 tor_addr_port_t *tor_addr_port_new(const tor_addr_t *addr, uint16_t port);
 
 #ifdef ADDRESS_PRIVATE
-STATIC smartlist_t *get_interface_addresses_raw(int severity);
+MOCK_DECL(smartlist_t *,get_interface_addresses_raw,(int severity));
 STATIC int tor_addr_is_multicast(const tor_addr_t *a);
-STATIC int get_interface_address6_via_udp_socket_hack(int severity,
-                                                      sa_family_t family,
-                                                      tor_addr_t *addr);
+MOCK_DECL(int,get_interface_address6_via_udp_socket_hack,(int severity,
+                                                          sa_family_t family,
+                                                          tor_addr_t *addr));
 
 #ifdef HAVE_IFADDRS_TO_SMARTLIST
 STATIC smartlist_t *ifaddrs_to_smartlist(const struct ifaddrs *ifa);

src/test/test_address.c

@@ -779,6 +779,99 @@ test_address_get_if_addrs6_list_no_internal(void *arg)
   return;
 }
 
+static int called_get_interface_addresses_raw = 0;
+
+smartlist_t *
+mock_get_interface_addresses_raw_fail(int severity)
+{
+  (void)severity;
+
+  called_get_interface_addresses_raw++;
+  return smartlist_new();
+}
+
+static int called_get_interface_address6_via_udp_socket_hack = 0;
+
+int
+mock_get_interface_address6_via_udp_socket_hack_fail(int severity,
+                                                     sa_family_t family,
+                                                     tor_addr_t *addr)
+{
+  (void)severity;
+  (void)family;
+  (void)addr;
+
+  called_get_interface_address6_via_udp_socket_hack++;
+  return -1;
+}
+
+static void
+test_address_get_if_addrs_internal_fail(void *arg)
+{
+  smartlist_t *results1 = NULL, *results2 = NULL;
+  int rv = 0;
+  uint32_t ipv4h_addr = 0;
+  tor_addr_t ipv6_addr;
+
+  memset(&ipv6_addr, 0, sizeof(tor_addr_t));
+
+  (void)arg;
+
+  MOCK(get_interface_addresses_raw,
+       mock_get_interface_addresses_raw_fail);
+  MOCK(get_interface_address6_via_udp_socket_hack,
+       mock_get_interface_address6_via_udp_socket_hack_fail);
+
+  results1 = get_interface_address6_list(LOG_ERR, AF_INET6, 1);
+  tt_assert(results1 != NULL);
+  tt_int_op(smartlist_len(results1),==,0);
+
+  results2 = get_interface_address_list(LOG_ERR, 1);
+  tt_assert(results2 != NULL);
+  tt_int_op(smartlist_len(results2),==,0);
+
+  rv = get_interface_address6(LOG_ERR, AF_INET6, &ipv6_addr);
+  tt_assert(rv == -1);
+
+  rv = get_interface_address(LOG_ERR, &ipv4h_addr);
+  tt_assert(rv == -1);
+
+done:
+  UNMOCK(get_interface_addresses_raw);
+  UNMOCK(get_interface_address6_via_udp_socket_hack);
+  free_interface_address6_list(results1);
+  free_interface_address6_list(results2);
+  return;
+}
+
+static void
+test_address_get_if_addrs_no_internal_fail(void *arg)
+{
+  smartlist_t *results1 = NULL, *results2 = NULL;
+
+  (void)arg;
+
+  MOCK(get_interface_addresses_raw,
+       mock_get_interface_addresses_raw_fail);
+  MOCK(get_interface_address6_via_udp_socket_hack,
+       mock_get_interface_address6_via_udp_socket_hack_fail);
+
+  results1 = get_interface_address6_list(LOG_ERR, AF_INET6, 0);
+  tt_assert(results1 != NULL);
+  tt_int_op(smartlist_len(results1),==,0);
+
+  results2 = get_interface_address_list(LOG_ERR, 0);
+  tt_assert(results2 != NULL);
+  tt_int_op(smartlist_len(results2),==,0);
+
+done:
+  UNMOCK(get_interface_addresses_raw);
+  UNMOCK(get_interface_address6_via_udp_socket_hack);
+  free_interface_address6_list(results1);
+  free_interface_address6_list(results2);
+  return;
+}
+
 static void
 test_address_get_if_addrs(void *arg)
 {
@@ -838,6 +931,8 @@ struct testcase_t address_tests[] = {
   ADDRESS_TEST(get_if_addrs_list_no_internal, 0),
   ADDRESS_TEST(get_if_addrs6_list_internal, 0),
   ADDRESS_TEST(get_if_addrs6_list_no_internal, 0),
+  ADDRESS_TEST(get_if_addrs_internal_fail, 0),
+  ADDRESS_TEST(get_if_addrs_no_internal_fail, 0),
   ADDRESS_TEST(get_if_addrs, 0),
   ADDRESS_TEST(get_if_addrs6, 0),
 #ifdef HAVE_IFADDRS_TO_SMARTLIST