On Tue, 11 Oct 2016 22:26:51 +0200, Daniel Jakots <danj+o...@chown.me> wrote:
> On Mon, 10 Oct 2016 21:46:54 +0200, Daniel Jakots <danj+o...@chown.me> > wrote: > > > Hi, > > > > This fixes CVE-2016-5180. ping > I had a look for -stable. The patch use a function that doesn't exist > in 1.10.0: > > > + buf = ares_malloc(len); > > I guess it appears in 1.11.0 because in the ChangeLog there is > > > Allow library-wide override of malloc/free > > So just backporting the diff doesn't work. Debian just > uses malloc in their backport (thanks olasd!): > https://sources.debian.net/src/c-ares/1.10.0-2%2Bdeb8u1/debian/patches/CVE-2016-5180.diff/ > > Doing the same thing and make package works. > > Comments? OK? > > Cheers, > Daniel > > Index: Makefile > =================================================================== > RCS file: /cvs/ports/net/libcares/Makefile,v > retrieving revision 1.16 > diff -u -p -r1.16 Makefile > --- Makefile 11 Mar 2016 19:59:15 -0000 1.16 > +++ Makefile 11 Oct 2016 20:26:07 -0000 > @@ -7,7 +7,7 @@ DISTNAME= c-ares-${V} > PKGNAME= libcares-${V} > CATEGORIES= net devel > MASTER_SITES= ${HOMEPAGE}download/ > -REVISION= 0 > +REVISION= 1 > > SHARED_LIBS= cares 2.5 > > Index: patches/patch-ares_create_query_c > =================================================================== > RCS file: patches/patch-ares_create_query_c > diff -N patches/patch-ares_create_query_c > --- /dev/null 1 Jan 1970 00:00:00 -0000 > +++ patches/patch-ares_create_query_c 11 Oct 2016 20:26:07 > -0000 @@ -0,0 +1,136 @@ > +$OpenBSD$ > + > +Patch for CVE-2016-5180 https://c-ares.haxx.se/adv_20160929.html > + > +--- ares_create_query.c.orig Wed Feb 13 11:01:50 2013 > ++++ ares_create_query.c Tue Oct 11 22:15:41 2016 > +@@ -85,57 +85,31 @@ > + */ > + > + int ares_create_query(const char *name, int dnsclass, int type, > +- unsigned short id, int rd, unsigned char > **buf, +- int *buflen, int max_udp_size) > ++ unsigned short id, int rd, unsigned char > **bufp, ++ int *buflenp, int max_udp_size) > + { > +- int len; > ++ size_t len; > + unsigned char *q; > + const char *p; > ++ size_t buflen; > ++ unsigned char *buf; > + > + /* Set our results early, in case we bail out early with an > error. */ +- *buflen = 0; > +- *buf = NULL; > ++ *buflenp = 0; > ++ *bufp = NULL; > + > +- /* Compute the length of the encoded name so we can check buflen. > +- * Start counting at 1 for the zero-length label at the end. */ > +- len = 1; > +- for (p = name; *p; p++) > +- { > +- if (*p == '\\' && *(p + 1) != 0) > +- p++; > +- len++; > +- } > +- /* If there are n periods in the name, there are n + 1 labels, and > +- * thus n + 1 length fields, unless the name is empty or ends > with a +- * period. So add 1 unless name is empty or ends with a > period. ++ /* Allocate a memory area for the maximum size this > packet might need. +2 ++ * is for the length byte and zero > termination if no dots or ecscaping is ++ * used. > + */ > +- if (*name && *(p - 1) != '.') > +- len++; > ++ len = strlen(name) + 2 + HFIXEDSZ + QFIXEDSZ + > ++ (max_udp_size ? EDNSFIXEDSZ : 0); > ++ buf = malloc(len); > ++ if (!buf) > ++ return ARES_ENOMEM; > + > +- /* Immediately reject names that are longer than the maximum of > 255 +- * bytes that's specified in RFC 1035 ("To simplify > implementations, +- * the total length of a domain name (i.e., > label octets and label +- * length octets) is restricted to 255 > octets or less."). We aren't +- * doing this just to be a stickler > about RFCs. For names that are +- * too long, 'dnscache' closes its > TCP connection to us immediately +- * (when using TCP) and ignores > the request when using UDP, and +- * BIND's named returns ServFail > (TCP or UDP). Sending a request +- * that we know will cause > 'dnscache' to close the TCP connection is +- * painful, since that > makes any other outstanding requests on that +- * connection fail. > And sending a UDP request that we know +- * 'dnscache' will ignore > is bad because resources will be tied up +- * until we time-out the > request. +- */ > +- if (len > MAXCDNAME) > +- return ARES_EBADNAME; > +- > +- *buflen = len + HFIXEDSZ + QFIXEDSZ + (max_udp_size ? > EDNSFIXEDSZ : 0); +- *buf = malloc(*buflen); > +- if (!*buf) > +- return ARES_ENOMEM; > +- > + /* Set up the header. */ > +- q = *buf; > ++ q = buf; > + memset(q, 0, HFIXEDSZ); > + DNS_HEADER_SET_QID(q, id); > + DNS_HEADER_SET_OPCODE(q, QUERY); > +@@ -159,8 +133,10 @@ int ares_create_query(const char *name, int > dnsclass, > + q += HFIXEDSZ; > + while (*name) > + { > +- if (*name == '.') > ++ if (*name == '.') { > ++ free (buf); > + return ARES_EBADNAME; > ++ } > + > + /* Count the number of bytes in this label. */ > + len = 0; > +@@ -170,8 +146,10 @@ int ares_create_query(const char *name, int > dnsclass, > + p++; > + len++; > + } > +- if (len > MAXLABEL) > ++ if (len > MAXLABEL) { > ++ free (buf); > + return ARES_EBADNAME; > ++ } > + > + /* Encode the length and copy the data. */ > + *q++ = (unsigned char)len; > +@@ -195,14 +173,30 @@ int ares_create_query(const char *name, int > dnsclass, > + DNS_QUESTION_SET_TYPE(q, type); > + DNS_QUESTION_SET_CLASS(q, dnsclass); > + > ++ q += QFIXEDSZ; > + if (max_udp_size) > + { > +- q += QFIXEDSZ; > + memset(q, 0, EDNSFIXEDSZ); > + q++; > + DNS_RR_SET_TYPE(q, T_OPT); > + DNS_RR_SET_CLASS(q, max_udp_size); > ++ q += (EDNSFIXEDSZ-1); > + } > ++ buflen = (q - buf); > ++ > ++ /* Reject names that are longer than the maximum of 255 bytes > that's ++ * specified in RFC 1035 ("To simplify implementations, > the total length of ++ * a domain name (i.e., label octets and > label length octets) is restricted ++ * to 255 octets or less."). */ > ++ if (buflen > (MAXCDNAME + HFIXEDSZ + QFIXEDSZ + > ++ (max_udp_size ? EDNSFIXEDSZ : 0))) { > ++ free (buf); > ++ return ARES_EBADNAME; > ++ } > ++ > ++ /* we know this fits in an int at this point */ > ++ *buflenp = (int) buflen; > ++ *bufp = buf; > + > + return ARES_SUCCESS; > + } >