| 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. |
| |
| |
| |
| |
| |
| @@ -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 (); |