On Mon, 10 Oct 2016 21:46:54 +0200, Daniel Jakots <danj+o...@chown.me> wrote:
> Hi, > > This fixes CVE-2016-5180. 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; + }
