Re: [PATCH] resolv: Always initialize *resplen2 out parameter [BZ #19791]

[Aurelien Jarno <aurelien at aurel32 dot net>]

On 2016-03-11 16:00, Florian Weimer wrote:
> Since commit 44d20bca52ace85850012b0ead37b360e3ecd96e (Implement
> second fallback mode for DNS requests), there is a code path
> which returns early, before *resplen2 is initialized.  This
> happens if the name server address is immediately recognized
> as invalid (because of lack of protocol support, or if it is
> a broadcast address such 255.255.255.255, or another invalid
> address).
> 
> If this happens and *resplen2 was non-zero (which is the case
> if a previous query resulted in a failure), __libc_res_nquery
> would reuse an existing second answer buffer.  This answer
> has been previously identified as unusable (for example,
> it could be an NXDOMAIN response).  Due to the presence of
> a second answer, no name server switching will occur.  The
> result is a name resolution failure, although a successful
> resolution would have been possible if name servers have
> been switched and queries had proceeded along the search path.
> 
> The above paragraph still simplifies the situation.  Before
> glibc 2.23, if the second answer needed malloc, the stub
> resolver would still attempt to reuse the second answer,
> but this is not possible because __libc_res_nsearch has freed
> it, after the unsuccessful call to __libc_res_nquerydomain,
> and set the buffer pointer to NULL.  This eventually leads to
> an assertion failure in __libc_res_nquery:
> 
> 	/* Make sure both hp and hp2 are defined */
> 	assert((hp != NULL) && (hp2 != NULL));
> 
> If assertions are disabled, the consequence is a NULL pointer
> dereference on the next line.
> 
> Starting with glibc 2.23, as a result of commit
> e9db92d3acfe1822d56d11abcea5bfc4c41cf6ca (CVE-2015-7547:
> getaddrinfo() stack-based buffer overflow (Bug 18665)),
> the second answer is always allocated with malloc.  This means
> that the assertion failure happens with small responses as well
> because there is no buffer to reuse, as soon as there is a
> name resolution failure which triggers a search for an answer
> along the search path.
> 
> I've also attached a revised test case (for the existing out-of-tree
> test framework).

Thanks for your analysis and your patch. It actually matches a failure
reported by a Debian user, which I was unable to reproduce so far:

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=816669

Aurelien

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net