Bug#839151: c-ares: CVE-2016-5180: out-of-bounds write in ares_create_query and ares_mkquery

2016-09-30 Thread Florian Weimer 
* Gregor Jasny:

> For Jessie I backported the patch and prepared an upload to
> jessie-security (debdiff also attached). Please review and tell me how
> to proceed.
>
> https://anonscm.debian.org/cgit/collab-maint/c-ares.git/log/?id=refs/heads/jessie
> https://mentors.debian.net/debian/pool/main/c/c-ares/c-ares_1.10.0-2+deb8u1.dsc

Thanks for working on this.

This looks good, I'll sponser an upload for you.

> Wheezy with c-ares 1.9.1 in not affected (stated in upstream announcement).

This is something the LTS folks may want to care of (see Daniel's
comment on the bug).

Bug#839151: c-ares: CVE-2016-5180: out-of-bounds write in ares_create_query and ares_mkquery

2016-09-29 Thread Daniel Stenberg 

On Thu, 29 Sep 2016, Gregor Jasny wrote:


Wheezy with c-ares 1.9.1 in not affected (stated in upstream announcement).


That isn't entirely accurate. This flaw is present in every c-ares release 
ever made until 1.12.0 unless you've done some changes not provided by 
upstream.


Before c-ares 1.10.0, the flaw is only present in the ares_mkquery() function 
which isn't identical to ares_create_query so the patch needs a little 
adjustment to apply there.


If you think I can clarify this better in the upstream advisory, please 
suggest a wording that would make it harder to misunderstand!


--

 / daniel.haxx.se

Bug#839151: c-ares: CVE-2016-5180: out-of-bounds write in ares_create_query and ares_mkquery

2016-09-29 Thread Gregor Jasny 
Hello Security Team,

This about https://security-tracker.debian.org/tracker/CVE-2016-5180 and
my first security upload. Please be gentle :)

For Sid and Stretch I uploaded 1.12.0-1 with urgency high.

For Jessie I backported the patch and prepared an upload to
jessie-security (debdiff also attached). Please review and tell me how
to proceed.

https://anonscm.debian.org/cgit/collab-maint/c-ares.git/log/?id=refs/heads/jessie
https://mentors.debian.net/debian/pool/main/c/c-ares/c-ares_1.10.0-2+deb8u1.dsc

Wheezy with c-ares 1.9.1 in not affected (stated in upstream announcement).

Thanks,
Gregor
diff -Nru c-ares-1.10.0/debian/changelog c-ares-1.10.0/debian/changelog
--- c-ares-1.10.0/debian/changelog  2013-06-16 13:39:20.0 +0200
+++ c-ares-1.10.0/debian/changelog  2016-09-29 20:30:48.0 +0200
@@ -1,3 +1,9 @@
+c-ares (1.10.0-2+deb8u1) jessie-security; urgency=high
+
+  * Apply patch for CVE-2016-5180 (Closes: #839151)
+
+ -- Gregor Jasny   Thu, 29 Sep 2016 20:30:48 +0200
+
 c-ares (1.10.0-2) unstable; urgency=low
 
   * Bump standards to v3.9.4 (no changes needed)
diff -Nru c-ares-1.10.0/debian/gbp.conf c-ares-1.10.0/debian/gbp.conf
--- c-ares-1.10.0/debian/gbp.conf   2012-05-10 21:24:25.0 +0200
+++ c-ares-1.10.0/debian/gbp.conf   2016-09-29 20:15:36.0 +0200
@@ -1,6 +1,6 @@
 [DEFAULT]
 upstream-branch = upstream
-debian-branch = master
+debian-branch = jessie
 upstream-tag = upstream/%(version)s
 debian-tag = debian/%(version)s
 pristine-tar = True
diff -Nru c-ares-1.10.0/debian/patches/CVE-2016-5180.diff 
c-ares-1.10.0/debian/patches/CVE-2016-5180.diff
--- c-ares-1.10.0/debian/patches/CVE-2016-5180.diff 1970-01-01 
01:00:00.0 +0100
+++ c-ares-1.10.0/debian/patches/CVE-2016-5180.diff 2016-09-29 
20:29:25.0 +0200
@@ -0,0 +1,138 @@
+From: Daniel Stenberg 
+Description: ares_create_query: avoid single-byte buffer overwrite 
(CVE-2016-5180)
+Origin: backport, https://c-ares.haxx.se/CVE-2016-5180.patch
+Bug-Debian: http://bugs.debian.org/839151
+
+
+--- a/ares_create_query.c
 b/ares_create_query.c
+@@ -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++;
+-
+-  /* 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;
++  len = strlen(name) + 2 + HFIXEDSZ + QFIXEDSZ +
++(max_udp_size ? EDNSFIXEDSZ : 0);
++  buf = malloc(len);
++  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 @@
+   q += HFIXEDSZ;
+   while (*name)
+ {
+-  if (*name == '.')
++  if (*name == '.') {
++

Bug#839151: c-ares: CVE-2016-5180: out-of-bounds write in ares_create_query and ares_mkquery

2016-09-29 Thread Florian Weimer 
Package: src:c-ares
Version: 1.11.0-1
Tags: security
Severity: important

A new upstream version of c-ares has been released which addresses a
security vulnerability.

From: Daniel Stenberg 
Date: Thu, 29 Sep 2016 16:02:10 +0200 (CEST)

`ares_create_query` single byte out of buffer write
=

Project c-ares Security Advisory, September 29, 2016 -
[Permalink](https://c-ares.haxx.se/adv_20160929.html)

VULNERABILITY
-

When a string is passed in to `ares_create_query` or `ares_mkquery` and uses
an escaped trailing dot, like "hello\.", c-ares calculates the string length
wrong and subsequently writes outside of the the allocated buffer with one
byte. The wrongly written byte is the least significant byte of the 'dnsclass'
argument; most commonly 1.

We have been seen proof of concept code showing how this can be exploited in a
real-world system, but we are not aware of any such instances having actually
happened in the wild.

INFO


The Common Vulnerabilities and Exposures (CVE) project has assigned the name
CVE-2016-5180 to this issue.

AFFECTED VERSIONS
-

This flaw exists in the following c-ares versions.

- Affected versions: libcurl 1.0.0 to and including 1.11.0
- Not affected versions: c-ares >= 1.12.0

[...]

A [patch for CVE-2016-5180](https://c-ares.haxx.se/CVE-2016-5180.patch) is
available.