From d277a9bbcdc27c48dff8f42e7f60413cef5de360 Mon Sep 17 00:00:00 2001 From: Florian Weimer Date: May 06 2016 15:23:21 +0000 Subject: Resolves: #1316972 resolv: Fix NULL pointer dereference with unconnectable address --- diff --git a/glibc-rh1316972-2.patch b/glibc-rh1316972-2.patch new file mode 100644 index 0000000..c7bf0b3 --- /dev/null +++ b/glibc-rh1316972-2.patch @@ -0,0 +1,117 @@ +commit b9bdfa7c8fa22c944bb5f21a673dfd1f91b71c56 +Author: Florian Weimer +Date: Wed Apr 27 14:26:47 2016 +0200 + + resolv: Always set *resplen2 out parameter in send_vc [BZ #19825] + + In various error scenarios (for example, if the server closes the + TCP connection before sending the full response), send_vc can return + without resetting the *resplen2 value. This can pass uninitialized + or unexpected data to the caller. + +Index: b/resolv/res_send.c +=================================================================== +--- a/resolv/res_send.c ++++ b/resolv/res_send.c +@@ -796,8 +796,6 @@ send_vc(res_state statp, + u_short len2; + u_char *cp; + +- if (resplen2 != NULL) +- *resplen2 = 0; + connreset = 0; + same_ns: + truncating = 0; +@@ -823,6 +821,8 @@ send_vc(res_state statp, + if (statp->_vcsock < 0) { + *terrno = errno; + Perror(statp, stderr, "socket(vc)", errno); ++ if (resplen2 != NULL) ++ *resplen2 = 0; + return (-1); + } + __set_errno (0); +@@ -833,8 +833,7 @@ send_vc(res_state statp, + *terrno = errno; + Aerror(statp, stderr, "connect/vc", errno, + (struct sockaddr *) nsap); +- __res_iclose(statp, false); +- return (0); ++ return close_and_return_error (statp, resplen2); + } + statp->_flags |= RES_F_VC; + } +@@ -857,8 +856,7 @@ send_vc(res_state statp, + if (TEMP_FAILURE_RETRY (writev(statp->_vcsock, iov, niov)) != explen) { + *terrno = errno; + Perror(statp, stderr, "write failed", errno); +- __res_iclose(statp, false); +- return (0); ++ return close_and_return_error (statp, resplen2); + } + /* + * Receive length & response +@@ -880,7 +878,6 @@ send_vc(res_state statp, + if (n <= 0) { + *terrno = errno; + Perror(statp, stderr, "read failed", errno); +- __res_iclose(statp, false); + /* + * A long running process might get its TCP + * connection reset if the remote server was +@@ -890,11 +887,13 @@ send_vc(res_state statp, + * instead of failing. We only allow one reset + * per query to prevent looping. + */ +- if (*terrno == ECONNRESET && !connreset) { +- connreset = 1; +- goto same_ns; +- } +- return (0); ++ if (*terrno == ECONNRESET && !connreset) ++ { ++ __res_iclose (statp, false); ++ connreset = 1; ++ goto same_ns; ++ } ++ return close_and_return_error (statp, resplen2); + } + int rlen = ntohs (rlen16); + +@@ -926,11 +925,11 @@ send_vc(res_state statp, + /* Always allocate MAXPACKET, callers expect + this specific size. */ + u_char *newp = malloc (MAXPACKET); +- if (newp == NULL) { +- *terrno = ENOMEM; +- __res_iclose(statp, false); +- return (0); +- } ++ if (newp == NULL) ++ { ++ *terrno = ENOMEM; ++ return close_and_return_error (statp, resplen2); ++ } + *thisanssizp = MAXPACKET; + *thisansp = newp; + if (thisansp == ansp2) +@@ -957,8 +956,7 @@ send_vc(res_state statp, + Dprint(statp->options & RES_DEBUG, + (stdout, ";; undersized: %d\n", len)); + *terrno = EMSGSIZE; +- __res_iclose(statp, false); +- return (0); ++ return close_and_return_error (statp, resplen2); + } + + cp = *thisansp; +@@ -969,8 +967,7 @@ send_vc(res_state statp, + if (__glibc_unlikely (n <= 0)) { + *terrno = errno; + Perror(statp, stderr, "read(vc)", errno); +- __res_iclose(statp, false); +- return (0); ++ return close_and_return_error (statp, resplen2); + } + if (__glibc_unlikely (truncating)) { + /* diff --git a/glibc-rh1316972.patch b/glibc-rh1316972.patch new file mode 100644 index 0000000..6cf28e9 --- /dev/null +++ b/glibc-rh1316972.patch @@ -0,0 +1,216 @@ +commit b66d837bb5398795c6b0f651bd5a5d66091d8577 +Author: Florian Weimer +Date: Fri Mar 25 11:49:51 2016 +0100 + + resolv: Always set *resplen2 out parameter in send_dg [BZ #19791] + + 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. + + This commit addresses the issue by ensuring that *resplen2 is + initialized before the send_dg function returns. + + This commit also addresses a bug where an invalid second reply is + incorrectly returned as a valid to the caller. + +Index: b/resolv/res_send.c +=================================================================== +--- a/resolv/res_send.c ++++ b/resolv/res_send.c +@@ -679,6 +679,18 @@ libresolv_hidden_def (res_nsend) + + /* Private */ + ++/* Close the resolver structure, assign zero to *RESPLEN2 if RESPLEN2 ++ is not NULL, and return zero. */ ++static int ++__attribute__ ((warn_unused_result)) ++close_and_return_error (res_state statp, int *resplen2) ++{ ++ __res_iclose(statp, false); ++ if (resplen2 != NULL) ++ *resplen2 = 0; ++ return 0; ++} ++ + /* The send_vc function is responsible for sending a DNS query over TCP + to the nameserver numbered NS from the res_state STATP i.e. + EXT(statp).nssocks[ns]. The function supports sending both IPv4 and +@@ -1183,7 +1195,11 @@ send_dg(res_state statp, + retry_reopen: + retval = reopen (statp, terrno, ns); + if (retval <= 0) +- return retval; ++ { ++ if (resplen2 != NULL) ++ *resplen2 = 0; ++ return retval; ++ } + retry: + evNowTime(&now); + evConsTime(&timeout, seconds, 0); +@@ -1196,8 +1212,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: +@@ -1205,9 +1219,7 @@ send_dg(res_state statp, + if (evCmpTime(finish, now) <= 0) { + poll_err_out: + Perror(statp, stderr, "poll", errno); +- err_out: +- __res_iclose(statp, false); +- return (0); ++ return close_and_return_error (statp, resplen2); + } + evSubTime(&timeout, &finish, &now); + need_recompute = 0; +@@ -1254,7 +1266,9 @@ send_dg(res_state statp, + } + + *gotsomewhere = 1; +- return (0); ++ if (resplen2 != NULL) ++ *resplen2 = 0; ++ return 0; + } + if (n < 0) { + if (errno == EINTR) +@@ -1322,7 +1336,7 @@ send_dg(res_state statp, + + fail_sendmmsg: + Perror(statp, stderr, "sendmmsg", errno); +- goto err_out; ++ return close_and_return_error (statp, resplen2); + } + } + else +@@ -1340,7 +1354,7 @@ send_dg(res_state statp, + if (errno == EINTR || errno == EAGAIN) + goto recompute_resend; + Perror(statp, stderr, "send", errno); +- goto err_out; ++ return close_and_return_error (statp, resplen2); + } + just_one: + if (nwritten != 0 || buf2 == NULL || single_request) +@@ -1418,7 +1432,7 @@ send_dg(res_state statp, + goto wait; + } + Perror(statp, stderr, "recvfrom", errno); +- goto err_out; ++ return close_and_return_error (statp, resplen2); + } + *gotsomewhere = 1; + if (__glibc_unlikely (*thisresplenp < HFIXEDSZ)) { +@@ -1429,7 +1443,7 @@ send_dg(res_state statp, + (stdout, ";; undersized: %d\n", + *thisresplenp)); + *terrno = EMSGSIZE; +- goto err_out; ++ return close_and_return_error (statp, resplen2); + } + if ((recvresp1 || hp->id != anhp->id) + && (recvresp2 || hp2->id != anhp->id)) { +@@ -1478,7 +1492,7 @@ send_dg(res_state statp, + ? *thisanssizp : *thisresplenp); + /* record the error */ + statp->_flags |= RES_F_EDNS0ERR; +- goto err_out; ++ return close_and_return_error (statp, resplen2); + } + #endif + if (!(statp->options & RES_INSECURE2) +@@ -1530,10 +1544,10 @@ send_dg(res_state statp, + goto wait; + } + +- __res_iclose(statp, false); + /* don't retry if called from dig */ + if (!statp->pfcode) +- return (0); ++ return close_and_return_error (statp, resplen2); ++ __res_iclose(statp, false); + } + if (anhp->rcode == NOERROR && anhp->ancount == 0 + && anhp->aa == 0 && anhp->ra == 0 && anhp->arcount == 0) { +@@ -1555,6 +1569,8 @@ send_dg(res_state statp, + __res_iclose(statp, false); + // XXX if we have received one reply we could + // XXX use it and not repeat it over TCP... ++ if (resplen2 != NULL) ++ *resplen2 = 0; + return (1); + } + /* Mark which reply we received. */ +@@ -1570,21 +1586,22 @@ send_dg(res_state statp, + __res_iclose (statp, false); + retval = reopen (statp, terrno, ns); + if (retval <= 0) +- return retval; ++ { ++ if (resplen2 != NULL) ++ *resplen2 = 0; ++ return retval; ++ } + pfd[0].fd = EXT(statp).nssocks[ns]; + } + } + goto wait; + } +- /* +- * All is well, or the error is fatal. Signal that the +- * next nameserver ought not be tried. +- */ ++ /* All is well. We have received both responses (if ++ two responses were requested). */ + return (resplen); +- } else if (pfd[0].revents & (POLLERR | POLLHUP | POLLNVAL)) { +- /* Something went wrong. We can stop trying. */ +- goto err_out; +- } ++ } else if (pfd[0].revents & (POLLERR | POLLHUP | POLLNVAL)) ++ /* Something went wrong. We can stop trying. */ ++ return close_and_return_error (statp, resplen2); + else { + /* poll should not have returned > 0 in this case. */ + abort (); diff --git a/glibc.spec b/glibc.spec index 0567393..73ca024 100644 --- a/glibc.spec +++ b/glibc.spec @@ -235,6 +235,8 @@ Patch1012: glibc-rh1313404-1.patch Patch1013: glibc-rh1313404-2.patch Patch1014: glibc-rh1313404-3.patch Patch1015: glibc-rh1321861.patch +Patch1016: glibc-rh1316972.patch +Patch1017: glibc-rh1316972-2.patch ############################################################################## # @@ -669,6 +671,8 @@ microbenchmark tests on the system. %patch1013 -p1 %patch1014 -p1 %patch1015 -p1 +%patch1016 -p1 +%patch1017 -p1 %patch0059 -p1 ############################################################################## @@ -1895,6 +1899,7 @@ rm -f *.filelist* - April 2016 nss_dns hardening (#1332914) - Fix elf/tst-audit10 and elf/tst-audit4 failures (#1313404) - nss_db: Fix handling of long entries (#1321861) +- resolv: Fix NULL pointer dereference with unconnectable address (#1316972) * Wed Mar 02 2016 Mike FABIAN - 2.22-11 - Add the C.UTF-8 locale