commit adb8538e7b6a7a2531233797990f6d11be86a718 Author: teor teor@torproject.org Date: Thu Jun 13 21:21:19 2019 +1000
address/resolve: Require square brackets on IPv6 address:ports
When parsing addreses via Tor's internal address:port parsing and DNS lookup APIs, require IPv6 addresses with ports to have square brackets.
But allow IPv6 addresses without ports, whether or not they have square brackets.
Fixes bug 30721; bugfix on 0.2.1.5-alpha. --- changes/bug30721 | 6 +++- src/lib/net/address.c | 77 ++++++++++++++++++++++++++++++++++++--------------- src/lib/net/resolve.c | 40 ++++++++++++++++++++++++-- 3 files changed, 96 insertions(+), 27 deletions(-)
diff --git a/changes/bug30721 b/changes/bug30721 index 5e0365300..5ea4a1462 100644 --- a/changes/bug30721 +++ b/changes/bug30721 @@ -1,6 +1,10 @@ - o Minor bugfixes (networking): + o Minor bugfixes (networking, IP addresses): - 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. + - When parsing addreses via Tor's internal address:port parsing and + DNS lookup APIs, require IPv6 addresses with ports to have square + brackets. But allow IPv6 addresses without ports, whether or not they + have square brackets. 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 75322ad88..0a2c84caf 100644 --- a/src/lib/net/address.c +++ b/src/lib/net/address.c @@ -1188,27 +1188,28 @@ fmt_addr32(uint32_t addr) }
/** Convert the string in <b>src</b> to a tor_addr_t <b>addr</b>. The string - * may be an IPv4 address, an IPv6 address, or an IPv6 address surrounded by - * square brackets. + * may be an IPv4 address, or an IPv6 address surrounded by square brackets. * - * Return an address family on success, or -1 if an invalid address string is - * provided. */ -int -tor_addr_parse(tor_addr_t *addr, const char *src) + * If <b>allow_ipv6_without_brackets</b> is true, also allow IPv6 addresses + * without brackets. + * + * Always rejects IPv4 addresses with brackets. + * + * Returns an address family on success, or -1 if an invalid address string is + * provided. */ +static int +tor_addr_parse_impl(tor_addr_t *addr, const char *src, + bool allow_ipv6_without_brackets) { /* Holds substring of IPv6 address after removing square brackets */ char *tmp = NULL; - int result; + int result = -1; struct in_addr in_tmp; struct in6_addr in6_tmp; int brackets_detected = 0;
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] == ']') { @@ -1216,21 +1217,46 @@ tor_addr_parse(tor_addr_t *addr, const char *src) src = tmp = tor_strndup(src+1, strlen(src)-2); }
- if (tor_inet_pton(AF_INET6, src, &in6_tmp) > 0) { - result = AF_INET6; - tor_addr_from_in6(addr, &in6_tmp); - } else if (!brackets_detected && - tor_inet_pton(AF_INET, src, &in_tmp) > 0) { - result = AF_INET; - tor_addr_from_in(addr, &in_tmp); - } else { - result = -1; + /* Try to parse an IPv6 address if it has brackets, or if IPv6 addresses + * without brackets are allowed */ + if (brackets_detected || allow_ipv6_without_brackets) { + if (tor_inet_pton(AF_INET6, src, &in6_tmp) > 0) { + result = AF_INET6; + tor_addr_from_in6(addr, &in6_tmp); + } + } + + /* Try to parse an IPv4 address without brackets */ + if (!brackets_detected) { + if (tor_inet_pton(AF_INET, src, &in_tmp) > 0) { + result = AF_INET; + tor_addr_from_in(addr, &in_tmp); + } + } + + /* Clear the address on error, to avoid returning uninitialised or partly + * parsed data. + */ + if (result == -1) { + memset(addr, 0, sizeof(tor_addr_t)); }
tor_free(tmp); return result; }
+/** Convert the string in <b>src</b> to a tor_addr_t <b>addr</b>. The string + * may be an IPv4 address, an IPv6 address, or an IPv6 address surrounded by + * square brackets. + * + * Returns an address family on success, or -1 if an invalid address string is + * provided. */ +int +tor_addr_parse(tor_addr_t *addr, const char *src) +{ + return tor_addr_parse_impl(addr, src, 1); +} + #ifdef HAVE_IFADDRS_TO_SMARTLIST /* * Convert a linked list consisting of <b>ifaddrs</b> structures @@ -1737,6 +1763,7 @@ tor_addr_port_parse(int severity, const char *addrport, int retval = -1; int r; char *addr_tmp = NULL; + bool has_port;
tor_assert(addrport); tor_assert(address_out); @@ -1746,15 +1773,19 @@ tor_addr_port_parse(int severity, const char *addrport, if (r < 0) goto done;
- if (!*port_out) { + has_port = !! *port_out; + /* If there's no port, use the default port, or fail if there is no default + */ + if (!has_port) { if (default_port >= 0) *port_out = default_port; else goto done; }
- /* make sure that address_out is an IP address */ - if (tor_addr_parse(address_out, addr_tmp) < 0) + /* Make sure that address_out is an IP address. + * If there is no port in addrport, allow IPv6 addresses without brackets. */ + if (tor_addr_parse_impl(address_out, addr_tmp, !has_port) < 0) goto done;
retval = 0; diff --git a/src/lib/net/resolve.c b/src/lib/net/resolve.c index 955572088..1097e36bd 100644 --- a/src/lib/net/resolve.c +++ b/src/lib/net/resolve.c @@ -258,32 +258,66 @@ tor_addr_port_lookup(const char *s, tor_addr_t *addr_out, uint16_t *port_out) uint16_t portval = 0; char *tmp = NULL; int rv = 0; + int result;
tor_assert(s); tor_assert(addr_out);
s = eat_whitespace(s);
+ /* Try parsing s as an address:port first, so we don't have to duplicate + * the logic that rejects IPv6:Port with no square brackets. */ + rv = tor_addr_port_parse(LOG_WARN, s, &addr, &portval, 0); + /* That was easy, no DNS required. */ + if (rv == 0) + goto success; + + /* Now let's check for malformed IPv6 addresses and ports: + * tor_addr_port_parse() requires squared brackes if there is a port, + * and we want tor_addr_port_lookup() to have the same requirement. + * But we strip the port using tor_addr_port_split(), so tor_addr_lookup() + * only sees the address, and will accept it without square brackets. */ + int family = tor_addr_parse(&addr, s); + /* If tor_addr_parse() succeeds where tor_addr_port_parse() failed, we need + * to reject this address as malformed. */ + if (family >= 0) { + /* Double-check it's an IPv6 address. If not, we have a parsing bug. + */ + tor_assertf_nonfatal(family == AF_INET6, + "Wrong family: %d (should be IPv6: %d) which " + "failed IP:port parsing, but passed IP parsing. " + "input string: '%s'; parsed address: '%s'.", + family, AF_INET6, s, fmt_addr(&addr)); + goto err; + } + + /* Now we have a hostname. Let's split off the port, if any. */ rv = tor_addr_port_split(LOG_WARN, s, &tmp, &portval); if (rv < 0) goto err;
+ /* And feed the hostname to the lookup function. */ if (tor_addr_lookup(tmp, AF_UNSPEC, &addr) != 0) goto err; - tor_free(tmp);
+ success: if (port_out) *port_out = portval; tor_addr_copy(addr_out, &addr); + result = 0; + goto done;
- return 0; err: /* Clear the address and port on error */ memset(addr_out, 0, sizeof(tor_addr_t)); if (port_out) *port_out = 0; + result = -1; + + /* We have set the result, now it's time to clean up */ + done: tor_free(tmp); - return -1; + return result; }
#ifdef USE_SANDBOX_GETADDRINFO