217 lines
7.2 KiB
Diff
217 lines
7.2 KiB
Diff
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: 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 ();
|