commit cd1de99468aa4d7b48c9f3bcb7c7d4fb5b3bfe9a Author: teor <teor@torproject.org> Date: Sun Jun 2 17:46:58 2019 +1000 resolve: consistently parse IP addresses in square brackets When parsing addreses via Tor's internal DNS lookup API: * reject IPv4 addresses in square brackets (with or without a port), * accept IPv6 addresses in square brackets (with or without a port), and * accept IPv6 addresses without square brackets, as long as they have no port. This change completes the work started in 23082, making address parsing consistent between tor's internal DNS lookup and address parsing APIs. Fixes bug 30721; bugfix on 0.2.1.5-alpha. --- changes/bug30721 | 6 +++++ src/lib/net/address.c | 31 +++++++++++++++++---- src/lib/net/resolve.c | 75 ++++++++++++++++++++++++++------------------------- 3 files changed, 70 insertions(+), 42 deletions(-) diff --git a/changes/bug30721 b/changes/bug30721 new file mode 100644 index 000000000..5e0365300 --- /dev/null +++ b/changes/bug30721 @@ -0,0 +1,6 @@ + o Minor bugfixes (networking): + - When parsing addreses via Tor's internal DNS lookup API, reject IPv4 + addresses in square brackets, and accept IPv6 addresses in square + brackets. This change completes the work started in 23082, making + address parsing consistent between tor's internal DNS lookup and address + parsing APIs. Fixes bug 30721; bugfix on 0.2.1.5-alpha. diff --git a/src/lib/net/address.c b/src/lib/net/address.c index 546af800a..4989e4ab2 100644 --- a/src/lib/net/address.c +++ b/src/lib/net/address.c @@ -373,7 +373,8 @@ tor_addr_to_str(char *dest, const tor_addr_t *addr, size_t len, int decorate) * * If <b>accept_regular</b> is set and the address is in neither recognized * reverse lookup hostname format, try parsing the address as a regular - * IPv4 or IPv6 address too. + * IPv4 or IPv6 address too. This mode will accept IPv6 addresses with or + * without square brackets. */ int tor_addr_parse_PTR_name(tor_addr_t *result, const char *address, @@ -1204,6 +1205,10 @@ tor_addr_parse(tor_addr_t *addr, const char *src) tor_assert(addr && src); + /* Clear the address before starting, to avoid returning uninitialised data. + */ + memset(addr, 0, sizeof(tor_addr_t)); + size_t len = strlen(src); if (len && src[0] == '[' && src[len - 1] == ']') { @@ -1718,6 +1723,11 @@ get_interface_address6_list,(int severity, * form "ip" or "ip:0". Otherwise, accept those forms, and set * *<b>port_out</b> to <b>default_port</b>. * + * This function accepts: + * - IPv6 address and port, when the IPv6 address is in square brackets, + * - IPv6 address with square brackets, + * - IPv6 address without square brackets. + * * Return 0 on success, -1 on failure. */ int tor_addr_port_parse(int severity, const char *addrport, @@ -1755,9 +1765,17 @@ tor_addr_port_parse(int severity, const char *addrport, } /** Given an address of the form "host[:port]", try to divide it into its host - * and port portions, setting *<b>address_out</b> to a newly allocated string - * holding the address portion and *<b>port_out</b> to the port (or 0 if no - * port is given). Return 0 on success, -1 on failure. */ + * and port portions. + * + * Like tor_addr_port_parse(), this function accepts: + * - IPv6 address and port, when the IPv6 address is in square brackets, + * - IPv6 address with square brackets, + * - IPv6 address without square brackets. + * + * Sets *<b>address_out</b> to a newly allocated string holding the address + * portion, and *<b>port_out</b> to the port (or 0 if no port is given). + * + * Return 0 on success, -1 on failure. */ int tor_addr_port_split(int severity, const char *addrport, char **address_out, uint16_t *port_out) @@ -1766,8 +1784,11 @@ tor_addr_port_split(int severity, const char *addrport, tor_assert(addrport); tor_assert(address_out); tor_assert(port_out); + /* We need to check for IPv6 manually because the logic below doesn't - * do a good job on IPv6 addresses that lack a port. */ + * do a good job on IPv6 addresses that lack a port. + * If an IPv6 address without square brackets is ambiguous, it gets parsed + * here as an address, rather than address:port. */ if (tor_addr_parse(&a_tmp, addrport) == AF_INET6) { *port_out = 0; *address_out = tor_strdup(addrport); diff --git a/src/lib/net/resolve.c b/src/lib/net/resolve.c index 49c263faa..71c033fe7 100644 --- a/src/lib/net/resolve.c +++ b/src/lib/net/resolve.c @@ -35,6 +35,8 @@ * *<b>addr</b> to the proper IP address, in host byte order. Returns 0 * on success, -1 on failure; 1 on transient failure. * + * This function only accepts IPv4 addresses. + * * (This function exists because standard windows gethostbyname * doesn't treat raw IP addresses properly.) */ @@ -62,6 +64,9 @@ tor_lookup_hostname,(const char *name, uint32_t *addr)) * <i>preferred</i> family, though another one may be returned if only one * family is implemented for this address. * + * Like tor_addr_parse(), this function accepts IPv6 addresses with or without + * square brackets. + * * Return 0 on success, -1 on failure; 1 on transient failure. */ MOCK_IMPL(int, @@ -70,26 +75,39 @@ tor_addr_lookup,(const char *name, uint16_t family, tor_addr_t *addr)) /* Perhaps eventually this should be replaced by a tor_getaddrinfo or * something. */ - struct in_addr iaddr; - struct in6_addr iaddr6; + int parsed_family = 0; + tor_assert(name); tor_assert(addr); tor_assert(family == AF_INET || family == AF_INET6 || family == AF_UNSPEC); + + /* Clear address before starting, to avoid returning uninitialised data */ + memset(addr, 0, sizeof(tor_addr_t)); + if (!*name) { /* Empty address is an error. */ return -1; - } else if (tor_inet_pton(AF_INET, name, &iaddr)) { + } + + /* Is it an IP address? */ + parsed_family = tor_addr_parse(addr, name); + + if (parsed_family == AF_INET) { /* It's an IPv4 IP. */ - if (family == AF_INET6) + if (family == AF_INET6) { + memset(addr, 0, sizeof(tor_addr_t)); return -1; - tor_addr_from_in(addr, &iaddr); + } return 0; - } else if (tor_inet_pton(AF_INET6, name, &iaddr6)) { - if (family == AF_INET) + } else if (parsed_family == AF_INET6) { + if (family == AF_INET) { + memset(addr, 0, sizeof(tor_addr_t)); return -1; - tor_addr_from_in6(addr, &iaddr6); + } return 0; } else { + /* Clear the address after a failed tor_addr_parse(). */ + memset(addr, 0, sizeof(tor_addr_t)); #ifdef HAVE_GETADDRINFO int err; struct addrinfo *res=NULL, *res_p; @@ -179,52 +197,35 @@ tor_addr_lookup,(const char *name, uint16_t family, tor_addr_t *addr)) /** Parse an address or address-port combination from <b>s</b>, resolve the * address as needed, and put the result in <b>addr_out</b> and (optionally) - * <b>port_out</b>. Return 0 on success, negative on failure. */ + * <b>port_out</b>. + * + * Like tor_addr_port_parse(), this function accepts: + * - IPv6 address and port, when the IPv6 address is in square brackets, + * - IPv6 address with square brackets, + * - IPv6 address without square brackets. + * + * Return 0 on success, negative on failure. */ int tor_addr_port_lookup(const char *s, tor_addr_t *addr_out, uint16_t *port_out) { - const char *port; tor_addr_t addr; uint16_t portval; char *tmp = NULL; + int rv = 0; tor_assert(s); tor_assert(addr_out); s = eat_whitespace(s); - if (*s == '[') { - port = strstr(s, "]"); - if (!port) - goto err; - tmp = tor_strndup(s+1, port-(s+1)); - port = port+1; - if (*port == ':') - port++; - else - port = NULL; - } else { - port = strchr(s, ':'); - if (port) - tmp = tor_strndup(s, port-s); - else - tmp = tor_strdup(s); - if (port) - ++port; - } + rv = tor_addr_port_split(LOG_WARN, s, &tmp, &portval); + if (rv < 0) + goto err; if (tor_addr_lookup(tmp, AF_UNSPEC, &addr) != 0) goto err; tor_free(tmp); - if (port) { - portval = (int) tor_parse_long(port, 10, 1, 65535, NULL, NULL); - if (!portval) - goto err; - } else { - portval = 0; - } - if (port_out) *port_out = portval; tor_addr_copy(addr_out, &addr);