| Message ID | 56E2DD9B.3060803@redhat.com |
|---|---|
| State | Superseded |
| Headers |
Received: (qmail 31654 invoked by alias); 11 Mar 2016 15:00:54 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <libc-alpha.sourceware.org> List-Unsubscribe: <mailto:libc-alpha-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:libc-alpha-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/libc-alpha/> List-Post: <mailto:libc-alpha@sourceware.org> List-Help: <mailto:libc-alpha-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 27299 invoked by uid 89); 11 Mar 2016 15:00:50 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=192, Before, 0x20, 0x01 X-HELO: mx1.redhat.com From: Florian Weimer <fweimer@redhat.com> Subject: [PATCH] resolv: Always initialize *resplen2 out parameter [BZ #19791] X-Enigmail-Draft-Status: N1110 To: GNU C Library <libc-alpha@sourceware.org> Message-ID: <56E2DD9B.3060803@redhat.com> Date: Fri, 11 Mar 2016 16:00:43 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="------------090400060206080300030609" |
Commit Message
Florian Weimer
March 11, 2016, 3 p.m. UTC
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). Florian
Comments
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
On 03/11/2016 05:41 PM, Aurelien Jarno wrote: > 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 Yes, this is the same bug, assuming that the reporter has disabled IPv6. Florian
On 03/11/2016 04:00 PM, 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). The patch is not as comprehensive as it should be. I'm preparing a replacement, which will also address bug 19825. Florian
2016-03-11 Florian Weimer <fweimer@redhat.com> [BZ #19791] * resolv/res_send.c (send_dg): Initialize *resplen2 early. diff --git a/resolv/res_send.c b/resolv/res_send.c index 25c19f1..5598ca6 100644 --- a/resolv/res_send.c +++ b/resolv/res_send.c @@ -1112,6 +1112,8 @@ send_dg(res_state statp, int retval; retry_reopen: + if (resplen2 != NULL) + *resplen2 = 0; retval = reopen (statp, terrno, ns); if (retval <= 0) return retval; @@ -1127,8 +1129,6 @@ send_dg(res_state statp, int recvresp2 = buf2 == NULL; pfd[0].fd = EXT(statp).nssocks[ns]; pfd[0].events = POLLOUT; - if (resplen2 != NULL) - *resplen2 = 0; wait: if (need_recompute) { recompute_resend: