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;
> + }
> 

Reply via email to