RFC6761 6.3 states:
The domain "localhost." and any names falling within ".localhost."
We were only honoring "localhost".
Fixes: #477
Fix By: Brad House (@bradh352)
In ares_set_sortlist, it calls config_sortlist(..., sortstr) to parse
the input str and initialize a sortlist configuration.
However, ares_set_sortlist has not any checks about the validity of the input str.
It is very easy to create an arbitrary length stack overflow with the unchecked
`memcpy(ipbuf, str, q-str);` and `memcpy(ipbufpfx, str, q-str);`
statements in the config_sortlist call, which could potentially cause severe
security impact in practical programs.
This commit add necessary check for `ipbuf` and `ipbufpfx` which avoid the
potential stack overflows.
fixes#496
Fix By: @hopper-vul
* add ares_strsplit unit test
The test reveals a bug in the implementation of ares_strsplit when the
make_set parameter is set to 1, as distinct domains are confused for
equal:
out = ares_strsplit("example.com, example.co", ", ", 1, &n);
evaluates to n = 1 with out = { "example.com" }.
* bugfix and cleanup of ares_strsplit
The purpose of ares_strsplit in c-ares is to split a comma-delimited
string of unique (up to letter case) domains. However, because the
terminating NUL byte was not checked in the substrings when comparing
for uniqueness, the function would sometimes drop domains it should
not. For example,
ares_strsplit("example.com, example.co", ",")
would only result in a single domain "example.com".
Aside from this bugfix, the following cleanup is performed:
1. The tokenization now happens with the help of strcspn instead of the
custom function is_delim.
2. The function list_contains has been inlined.
3. The interface of ares_strsplit has been simplified by removing the
parameter make_set since in practice it was always 1.
4. There are fewer passes over the input string.
5. We resize the table using realloc() down to its minimum size.
6. The docstring of ares_strsplit is updated and also a couple typos
are fixed.
There occurs a single use of ares_strsplit and since the make_set
parameter has been removed, the call in ares_init.c is modified
accordingly. The unit test for ares_strsplit is also updated.
Fix By: Nikolaos Chatzikonstantinou (@createyourpersonalaccount)
ai_addrlen was erroneously returning 16 bytes instead of the
sizeof(struct sockaddr_in6). This is a regression introduced
in 1.18.0.
Reported by: James Brown <jbrown@easypost.com>
Fix By: Brad House (@bradh352)
As per RFC6761 Section 6.3, "localhost" lookups need to be special cased to return loopback addresses, and not forward queries to recursive dns servers.
We first look up via files (/etc/hosts or equivalent), and if that fails, we then attempt a system-specific address enumeration for loopback addresses (currently Windows-only), and finally fallback to ::1 and 127.0.0.1.
Fix By: Brad House (@bradh352)
Fixes Bug: #399
ares_gethostbyname() and ares_getaddrinfo() do a lot of similar things, however ares_getaddrinfo() has some desirable behaviors that should be imported into ares_gethostbyname(). For one, it sorts the address lists for the most likely to succeed based on the current system routes. Next, when AF_UNSPEC is specified, it properly handles search lists instead of first searching all of AF_INET6 then AF_INET, since ares_gethostbyname() searches in parallel. Therefore, this PR should also resolve the issues attempted in #94.
A few things this PR does:
1. ares_parse_a_reply() and ares_parse_aaaa_reply() had very similar code to translate struct ares_addrinfo into a struct hostent as well as into struct ares_addrttl/ares_addr6ttl this has been split out into helper functions of ares__addrinfo2hostent() and ares__addrinfo2addrttl() to prevent this duplicative code.
2. ares_getaddrinfo() was apparently never honoring HOSTALIASES, and this was discovered once ares_gethostbyname() was turned into a wrapper, the affected test cases started failing.
3. A slight API modification to save the query hostname into struct ares_addrinfo as the last element of name. Since this is the last element, and all user-level instances of struct ares_addrinfo are allocated internally by c-ares, this is not an ABI-breaking change nor would it impact any API compatibility. This was needed since struct hostent has an h_name element.
4. Test Framework: MockServer tests via TCP would fail if more than 1 request was received at a time which is common when ares_getaddrinfo() queries for both A and AAAA records simultaneously. Infact, this was a long standing issue in which the ares_getaddrinfo() test were bypassing TCP alltogether. This has been corrected, the message is now processed in a loop.
5. Some tests had to be updated for overall correctness as they were invalid but somehow passing prior to this change.
Change By: Brad House (@bradh352)
ax_code_coverage.m4 dropped the @CODE_COVERAGE_RULES@ macro, so we need to switch to the latest recommendation from the m4 file. This requires updates to Makefile.am.
Fix By: Felix Yan (@felixonmars)
To prevent possible users having XSS issues due to intentionally malformed DNS replies, validate hostnames returned in responses and return EBADRESP if they are not valid.
It is not clear what legitimate issues this may cause at this point.
Bug Reported By: philipp.jeitner@sit.fraunhofer.de
Fix By: Brad House (@bradh352)
There is too much inconsistency between platforms for arpa/nameser.h and arpa/nameser_compat.h for the way the current files are structured. Still load the respective system files but make our private nameser.h more forgiving.
Fixes: #388
Fix By: Brad House (@bradh352)
Some systems may return either NULL or a valid pointer on malloc(0). c-ares should never call malloc(0) so lets return NULL so we're more likely to find an issue if it were to occur.
It appears that when building tests, it would hardcode enabling building
of the c-ares static library. This was probably due to Windows limitations
in symbol visibility.
This change will use the static library if it exists for tests, always.
Otherwise, it will only forcibly enable static libraries for tests on
Windows.
Fixes: #380
Fix By: Brad House (@bradh352)
The fuzz test files were not being distributed. This doesn't appear to be
a regression, it looks like they have never been distributed.
Fixes: #379
Fix By: Brad House (@bradh352)
CAA (Certification Authority Authorization) was introduced in RFC 6844.
This has been obsoleted by RFC 8659. This commit added the possibility
to query CAA resource records with adig and adds a parser for CAA
records, that can be used in conjunction with ares_query(3).
Closes Bug: #292
Fix By: Daniela Sonnenschein (@lxdicted)
Originally started by Daniel Stenberg (@bagder) with #123, this patch reorganizes the c-ares source tree to have a more modern layout. It also fixes out of tree builds for autotools, and automatically builds the tests if tests are enabled. All tests are passing which tests each of the supported build systems (autotools, cmake, nmake, mingw gmake). There may be some edge cases that will have to be caught later on for things I'm not aware of.
Fix By: Brad House (@bradh352)
EDNS retry should be based on FORMERR returned without an OPT RR record as per https://tools.ietf.org/html/rfc6891#section-7 rather than just treating any unexpected error condition as a reason to disable EDNS on the channel.
Fix By: Erik Lax (@eriklax)
The c-ares test suite was hardcoded to use port 5300 (and possibly 5301, 5302) for the test suite. Especially in containers, there may be no guarantee these ports are available and cause tests to fail when they could otherwise succeed. Instead, request the system to assign a port to use dynamically. This is now the default. To override, the test suite still takes the "-p <port>" option as it always has and will honor that.
Fix By: Anthony Penniston (@apenn-msft)
If a query is performed for dynamodb.us-east-1.amazonaws.com. with ndots=5, it was attempting to search the search domains rather than just attempting the FQDN that was passed it. This patch now at least attempts the FQDN first.
We may need to determine if we should abort any further searching, however as is probably intended.
Fix by: Jonathan Maye-Hobbs (@wheelpharoah)
Test cases where not honoring the HAVE_WRITEV flag but instead using WIN32 to determine if WRITEV was available or not. This patch fixes that.
Fix By: Bulat Gaifullin (@bgaifullin)
The rc4 function iterates over a buffer of size buffer_len who's maximum
value is INT_MAX with a counter of type short that is not guaranteed to
have maximum size INT_MAX.
In circumstances where short is narrower than int and where buffer_len
is larger than the maximum value of a short, it may be possible to loop
infinitely as counter will overflow and never be greater than or equal
to buffer_len.
The solution is to make the comparison be between types of equal width.
This commit defines counter as an int.
Fix By: Fionn Fitzmaurice (@fionn)
Jay Freeman (saurik)
bradh352
hopper-vul
Nikolaos Chatzikonstantinou
Manish Mehra
Felix Yan
Martin Holeš
David Hotham
Daniela Sonnenschein
Erik Lax
apenn-msft
Jonathan Maye-Hobbs
Bulat Gaifullin
Fionn Fitzmaurice