You can not select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
216 lines
7.2 KiB
216 lines
7.2 KiB
commit b66d837bb5398795c6b0f651bd5a5d66091d8577 |
|
Author: Florian Weimer <fweimer@redhat.com> |
|
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: glibc-2.17-c758a686/resolv/res_send.c |
|
=================================================================== |
|
--- glibc-2.17-c758a686.orig/resolv/res_send.c |
|
+++ glibc-2.17-c758a686/resolv/res_send.c |
|
@@ -672,6 +672,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 |
|
@@ -1159,7 +1171,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); |
|
@@ -1172,8 +1188,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: |
|
@@ -1181,9 +1195,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; |
|
@@ -1230,7 +1242,9 @@ send_dg(res_state statp, |
|
} |
|
|
|
*gotsomewhere = 1; |
|
- return (0); |
|
+ if (resplen2 != NULL) |
|
+ *resplen2 = 0; |
|
+ return 0; |
|
} |
|
if (n < 0) { |
|
if (errno == EINTR) |
|
@@ -1298,7 +1312,7 @@ send_dg(res_state statp, |
|
|
|
fail_sendmmsg: |
|
Perror(statp, stderr, "sendmmsg", errno); |
|
- goto err_out; |
|
+ return close_and_return_error (statp, resplen2); |
|
} |
|
} |
|
else |
|
@@ -1316,7 +1330,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) |
|
@@ -1392,7 +1406,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 (__builtin_expect (*thisresplenp < HFIXEDSZ, 0)) { |
|
@@ -1403,7 +1417,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)) { |
|
@@ -1452,7 +1466,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) |
|
@@ -1504,10 +1518,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) { |
|
@@ -1529,6 +1543,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. */ |
|
@@ -1544,21 +1560,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 ();
|
|
|