commit 7fa102b48783b51673095e1ddcb2f88050a2ba32 Author: teor (Tim Wilson-Brown) teor2345@gmail.com Date: Tue Sep 29 07:04:49 2015 +0200

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 | 10 +++++ src/common/address.c | 24 ++++++---- src/common/address.h | 8 ++-- src/test/test_address.c | 95 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 125 insertions(+), 12 deletions(-)

diff --git a/changes/bug17173-socket-hack-rv b/changes/bug17173-socket-hack-rv new file mode 100644 index 0000000..c9f30d3 --- /dev/null +++ b/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. diff --git a/src/common/address.c b/src/common/address.c index fc85f8e..cfa8fd1 100644 --- a/src/common/address.c +++ b/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); diff --git a/src/common/address.h b/src/common/address.h index 7d49fb5..d2841e1 100644 --- a/src/common/address.h +++ b/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); diff --git a/src/test/test_address.c b/src/test/test_address.c index 72742df..b442f4a 100644 --- a/src/test/test_address.c +++ b/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