| Message ID | 56FA607D.4070803@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 03/29/2016 07:01 AM, Florian Weimer wrote: > This is a minor security issue in nss_dns, triggered by a very long name > passed to getnetbyname. > > The defensive copy is not needed because the name may not alias the > output buffer. This code dates back to the original 1996 sources that were included in glibc and I expect that at that point the propagation of `const char *` for name was not entirely complete and that at some point the internals assumed they could scribble on name. That is not the case today, we propagate `const char *` to all callers, and a rather deep review that I just did using ctags shows that the call tree preserves the constness of name. This patch looks good to me. > 2016-03-29 Florian Weimer <fweimer@redhat.com> > > [BZ #19879] > CVE-2016-3075 > * resolv/nss_dns/dns-network.c (_nss_dns_getnetbyname_r): Do not > copy name. > > diff --git a/resolv/nss_dns/dns-network.c b/resolv/nss_dns/dns-network.c > index 2eb2f67..8f301a7 100644 > --- a/resolv/nss_dns/dns-network.c > +++ b/resolv/nss_dns/dns-network.c > @@ -118,17 +118,14 @@ _nss_dns_getnetbyname_r (const char *name, struct netent *result, > } net_buffer; > querybuf *orig_net_buffer; > int anslen; > - char *qbuf; > enum nss_status status; > > if (__res_maybe_init (&_res, 0) == -1) > return NSS_STATUS_UNAVAIL; > > - qbuf = strdupa (name); > - > net_buffer.buf = orig_net_buffer = (querybuf *) alloca (1024); > > - anslen = __libc_res_nsearch (&_res, qbuf, C_IN, T_PTR, net_buffer.buf->buf, > + anslen = __libc_res_nsearch (&_res, name, C_IN, T_PTR, net_buffer.buf->buf, > 1024, &net_buffer.ptr, NULL, NULL, NULL, NULL); > if (anslen < 0) > {
On Tue, 29 Mar 2016, Florian Weimer wrote: > This is a minor security issue in nss_dns, triggered by a very long name > passed to getnetbyname. As a security issue it should have an entry in the "Security related changes" section of NEWS for 2.24.
On 03/29/2016 11:41 PM, Joseph Myers wrote: > On Tue, 29 Mar 2016, Florian Weimer wrote: > >> This is a minor security issue in nss_dns, triggered by a very long name >> passed to getnetbyname. > > As a security issue it should have an entry in the "Security related > changes" section of NEWS for 2.24. I expected to wait with this until closer to the 2.24, but I guess there is no harm in adding these entries now. I pushed the following NEWS entry: * An unnecessary stack copy in _nss_dns_getnetbyname_r was removed. It could result in a stack overflow when getnetbyname was called with an overly long name. (CVE-2016-3075) Thanks, Florian
2016-03-29 Florian Weimer <fweimer@redhat.com> [BZ #19879] CVE-2016-3075 * resolv/nss_dns/dns-network.c (_nss_dns_getnetbyname_r): Do not copy name. diff --git a/resolv/nss_dns/dns-network.c b/resolv/nss_dns/dns-network.c index 2eb2f67..8f301a7 100644 --- a/resolv/nss_dns/dns-network.c +++ b/resolv/nss_dns/dns-network.c @@ -118,17 +118,14 @@ _nss_dns_getnetbyname_r (const char *name, struct netent *result, } net_buffer; querybuf *orig_net_buffer; int anslen; - char *qbuf; enum nss_status status; if (__res_maybe_init (&_res, 0) == -1) return NSS_STATUS_UNAVAIL; - qbuf = strdupa (name); - net_buffer.buf = orig_net_buffer = (querybuf *) alloca (1024); - anslen = __libc_res_nsearch (&_res, qbuf, C_IN, T_PTR, net_buffer.buf->buf, + anslen = __libc_res_nsearch (&_res, name, C_IN, T_PTR, net_buffer.buf->buf, 1024, &net_buffer.ptr, NULL, NULL, NULL, NULL); if (anslen < 0) {