[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#1036592: pre-approval: unblock: c-ares/1.18.1-3



Hi Gregor,

On Tue, May 23, 2023 at 08:44:48AM +0200, Gregor Jasny wrote:
> Package: release.debian.org
> Severity: normal
> User: release.debian.org@packages.debian.org
> Usertags: unblock
> X-Debbugs-Cc: c-ares@packages.debian.org
> Control: affects -1 + src:c-ares
> 
> Hello,
> 
> [ Reason ]
> 
> yesterday a version 1.19.1 of c-ares was release which fixes four CVEs.
> The Debian Security team considers two of them relevant for Debian and
> I'd like to cherry-pick them into the unstable package so that the fixes
> can migrate to Bookworm.
> 
> Attached you'll find the debdiff. The changes are also visible in Salsa:
> https://salsa.debian.org/debian/c-ares/-/compare/debian%2F1.18.1-2...master?from_project_id=11264&straight=false
> 
> [ Impact ]
> 
> CVE-2023-31130 has a CVSS score of 4.1
> CVE-2023-32067 has a CVSS score of 7.5
> 
> [ Tests ]
> 
> On the experimental branch I enabled the unit and integration tests:
> would you consider that commit as acceptable, too?
> https://salsa.debian.org/debian/c-ares/-/commit/25f515f728eeae82013a9c1cb8aa6ce80e913d09
> 
> [ Risks ]
> 
> The fix for the 0-byte DoS issue seems to be straight-forward.
> The fix for inet_net_pton_ipv6 has been synced from OpenBSD and
> is covered by the unit tests.
> 
> Both changes are port of the 1.19.1 release which built and passed
> tests on experimental (except Hurd):
> https://buildd.debian.org/status/package.php?p=c-ares&suite=experimental
> 
> [ Checklist ]
>   [x] all changes are documented in the d/changelog
>   [x] I reviewed all changes and I approve them
>   [x] attach debdiff against the package in testing
> 
> unblock c-ares/1.18.1-3

Glad to see you worked on it already. I was on it today to propose a
NMU, due to the deadline for bookworm approaching quickly, until
Moritz pointed out to me that you did already filled a unblock
request pre-approval.

Attached for reference what I did, and so they match. Release team,
can you accept it as we would like to see as well a bullseye-security
upload for the same two CVEs and avoid a regression
bullseye->bookworm?

Leaving open the question on enabling the testsuite.

Regards,
Salvatore
diff -Nru c-ares-1.18.1/debian/changelog c-ares-1.18.1/debian/changelog
--- c-ares-1.18.1/debian/changelog	2023-02-17 23:34:35.000000000 +0100
+++ c-ares-1.18.1/debian/changelog	2023-05-23 14:34:52.000000000 +0200
@@ -1,3 +1,11 @@
+c-ares (1.18.1-2.1) unstable; urgency=high
+
+  * Non-maintainer upload.
+  * Buffer Underwrite in ares_inet_net_pton() (CVE-2023-31130)
+  * 0-byte UDP payload Denial of Service (CVE-2023-32067)
+
+ -- Salvatore Bonaccorso <carnil@debian.org>  Tue, 23 May 2023 14:34:52 +0200
+
 c-ares (1.18.1-2) unstable; urgency=medium
 
   * Add str len check in config_sortlist to avoid stack overflow
diff -Nru c-ares-1.18.1/debian/patches/CVE-2023-31130.diff c-ares-1.18.1/debian/patches/CVE-2023-31130.diff
--- c-ares-1.18.1/debian/patches/CVE-2023-31130.diff	1970-01-01 01:00:00.000000000 +0100
+++ c-ares-1.18.1/debian/patches/CVE-2023-31130.diff	2023-05-23 14:34:52.000000000 +0200
@@ -0,0 +1,325 @@
+From: Brad House <brad@brad-house.com>
+Date: Mon, 22 May 2023 06:51:34 -0400
+Subject: Merge pull request from GHSA-x6mf-cxr9-8q6v
+Origin: https://github.com/c-ares/c-ares/commit/f22cc01039b6473b736d3bf438f56a2654cdf2b2
+Bug-Debian-Security: https://security-tracker.debian.org/tracker/CVE-2023-31130
+
+* Merged latest OpenBSD changes for inet_net_pton_ipv6() into c-ares.
+* Always use our own IP conversion functions now, do not delegate to OS
+  so we can have consistency in testing and fuzzing.
+* Removed bogus test cases that never should have passed.
+* Add new test case for crash bug found.
+
+Fix By: Brad House (@bradh352)
+---
+ src/lib/inet_net_pton.c    | 155 ++++++++++++++++++++-----------------
+ test/ares-test-internal.cc |   7 +-
+ 2 files changed, 86 insertions(+), 76 deletions(-)
+
+diff --git a/src/lib/inet_net_pton.c b/src/lib/inet_net_pton.c
+index 840de5065290..fc50425b8ea2 100644
+--- a/src/lib/inet_net_pton.c
++++ b/src/lib/inet_net_pton.c
+@@ -1,19 +1,20 @@
+ 
+ /*
+- * Copyright (c) 2004 by Internet Systems Consortium, Inc. ("ISC")
++ * Copyright (c) 2012 by Gilles Chehade <gilles@openbsd.org>
+  * Copyright (c) 1996,1999 by Internet Software Consortium.
+  *
+  * Permission to use, copy, modify, and distribute this software for any
+  * purpose with or without fee is hereby granted, provided that the above
+  * copyright notice and this permission notice appear in all copies.
+  *
+- * THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES
+- * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+- * MERCHANTABILITY AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR
+- * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+- * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+- * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT
+- * OF OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
++ * THE SOFTWARE IS PROVIDED "AS IS" AND INTERNET SOFTWARE CONSORTIUM DISCLAIMS
++ * ALL WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES
++ * OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL INTERNET SOFTWARE
++ * CONSORTIUM BE LIABLE FOR ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL
++ * DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR
++ * PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS
++ * ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS
++ * SOFTWARE.
+  */
+ 
+ #include "ares_setup.h"
+@@ -35,9 +36,6 @@
+ 
+ const struct ares_in6_addr ares_in6addr_any = { { { 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0 } } };
+ 
+-
+-#ifndef HAVE_INET_NET_PTON
+-
+ /*
+  * static int
+  * inet_net_pton_ipv4(src, dst, size)
+@@ -60,7 +58,7 @@ const struct ares_in6_addr ares_in6addr_any = { { { 0,0,0,0,0,0,0,0,0,0,0,0,0,0,
+  *      Paul Vixie (ISC), June 1996
+  */
+ static int
+-inet_net_pton_ipv4(const char *src, unsigned char *dst, size_t size)
++ares_inet_net_pton_ipv4(const char *src, unsigned char *dst, size_t size)
+ {
+   static const char xdigits[] = "0123456789abcdef";
+   static const char digits[] = "0123456789";
+@@ -261,19 +259,14 @@ getv4(const char *src, unsigned char *dst, int *bitsp)
+ }
+ 
+ static int
+-inet_net_pton_ipv6(const char *src, unsigned char *dst, size_t size)
++ares_inet_pton6(const char *src, unsigned char *dst)
+ {
+   static const char xdigits_l[] = "0123456789abcdef",
+-    xdigits_u[] = "0123456789ABCDEF";
++        xdigits_u[] = "0123456789ABCDEF";
+   unsigned char tmp[NS_IN6ADDRSZ], *tp, *endp, *colonp;
+   const char *xdigits, *curtok;
+-  int ch, saw_xdigit;
++  int ch, saw_xdigit, count_xdigit;
+   unsigned int val;
+-  int digits;
+-  int bits;
+-  size_t bytes;
+-  int words;
+-  int ipv4;
+ 
+   memset((tp = tmp), '\0', NS_IN6ADDRSZ);
+   endp = tp + NS_IN6ADDRSZ;
+@@ -283,22 +276,22 @@ inet_net_pton_ipv6(const char *src, unsigned char *dst, size_t size)
+     if (*++src != ':')
+       goto enoent;
+   curtok = src;
+-  saw_xdigit = 0;
++  saw_xdigit = count_xdigit = 0;
+   val = 0;
+-  digits = 0;
+-  bits = -1;
+-  ipv4 = 0;
+   while ((ch = *src++) != '\0') {
+     const char *pch;
+ 
+     if ((pch = strchr((xdigits = xdigits_l), ch)) == NULL)
+       pch = strchr((xdigits = xdigits_u), ch);
+     if (pch != NULL) {
++      if (count_xdigit >= 4)
++        goto enoent;
+       val <<= 4;
+-      val |= aresx_sztoui(pch - xdigits);
+-      if (++digits > 4)
++      val |= (pch - xdigits);
++      if (val > 0xffff)
+         goto enoent;
+       saw_xdigit = 1;
++      count_xdigit++;
+       continue;
+     }
+     if (ch == ':') {
+@@ -308,78 +301,107 @@ inet_net_pton_ipv6(const char *src, unsigned char *dst, size_t size)
+           goto enoent;
+         colonp = tp;
+         continue;
+-      } else if (*src == '\0')
++      } else if (*src == '\0') {
+         goto enoent;
++      }
+       if (tp + NS_INT16SZ > endp)
+-        return (0);
+-      *tp++ = (unsigned char)((val >> 8) & 0xff);
+-      *tp++ = (unsigned char)(val & 0xff);
++        goto enoent;
++      *tp++ = (unsigned char) (val >> 8) & 0xff;
++      *tp++ = (unsigned char) val & 0xff;
+       saw_xdigit = 0;
+-      digits = 0;
++      count_xdigit = 0;
+       val = 0;
+       continue;
+     }
+     if (ch == '.' && ((tp + NS_INADDRSZ) <= endp) &&
+-        getv4(curtok, tp, &bits) > 0) {
+-      tp += NS_INADDRSZ;
++        ares_inet_net_pton_ipv4(curtok, tp, INADDRSZ) > 0) {
++      tp += INADDRSZ;
+       saw_xdigit = 0;
+-      ipv4 = 1;
++      count_xdigit = 0;
+       break;  /* '\0' was seen by inet_pton4(). */
+     }
+-    if (ch == '/' && getbits(src, &bits) > 0)
+-      break;
+     goto enoent;
+   }
+   if (saw_xdigit) {
+     if (tp + NS_INT16SZ > endp)
+       goto enoent;
+-    *tp++ = (unsigned char)((val >> 8) & 0xff);
+-    *tp++ = (unsigned char)(val & 0xff);
++    *tp++ = (unsigned char) (val >> 8) & 0xff;
++    *tp++ = (unsigned char) val & 0xff;
+   }
+-  if (bits == -1)
+-    bits = 128;
+-
+-  words = (bits + 15) / 16;
+-  if (words < 2)
+-    words = 2;
+-  if (ipv4)
+-    words = 8;
+-  endp =  tmp + 2 * words;
+-
+   if (colonp != NULL) {
+     /*
+      * Since some memmove()'s erroneously fail to handle
+      * overlapping regions, we'll do the shift by hand.
+      */
+-    const ares_ssize_t n = tp - colonp;
+-    ares_ssize_t i;
++    const int n = tp - colonp;
++    int i;
+ 
+     if (tp == endp)
+       goto enoent;
+     for (i = 1; i <= n; i++) {
+-      *(endp - i) = *(colonp + n - i);
+-      *(colonp + n - i) = 0;
++      endp[- i] = colonp[n - i];
++      colonp[n - i] = 0;
+     }
+     tp = endp;
+   }
+   if (tp != endp)
+     goto enoent;
+ 
+-  bytes = (bits + 7) / 8;
+-  if (bytes > size)
+-    goto emsgsize;
+-  memcpy(dst, tmp, bytes);
+-  return (bits);
++  memcpy(dst, tmp, NS_IN6ADDRSZ);
++  return (1);
+ 
+-  enoent:
++enoent:
+   SET_ERRNO(ENOENT);
+   return (-1);
+ 
+-  emsgsize:
++emsgsize:
+   SET_ERRNO(EMSGSIZE);
+   return (-1);
+ }
+ 
++static int
++ares_inet_net_pton_ipv6(const char *src, unsigned char *dst, size_t size)
++{
++  struct ares_in6_addr in6;
++  int                  ret;
++  int                  bits;
++  size_t               bytes;
++  char                 buf[INET6_ADDRSTRLEN + sizeof("/128")];
++  char                *sep;
++  const char          *errstr;
++
++  if (strlen(src) >= sizeof buf) {
++    SET_ERRNO(EMSGSIZE);
++    return (-1);
++  }
++  strncpy(buf, src, sizeof buf);
++
++  sep = strchr(buf, '/');
++  if (sep != NULL)
++    *sep++ = '\0';
++
++  ret = ares_inet_pton6(buf, (unsigned char *)&in6);
++  if (ret != 1)
++    return (-1);
++
++  if (sep == NULL)
++    bits = 128;
++  else {
++    if (!getbits(sep, &bits)) {
++      SET_ERRNO(ENOENT);
++      return (-1);
++    }
++  }
++
++  bytes = (bits + 7) / 8;
++  if (bytes > size) {
++    SET_ERRNO(EMSGSIZE);
++    return (-1);
++  }
++  memcpy(dst, &in6, bytes);
++  return (bits);
++}
++
+ /*
+  * int
+  * inet_net_pton(af, src, dst, size)
+@@ -403,18 +425,15 @@ ares_inet_net_pton(int af, const char *src, void *dst, size_t size)
+ {
+   switch (af) {
+   case AF_INET:
+-    return (inet_net_pton_ipv4(src, dst, size));
++    return (ares_inet_net_pton_ipv4(src, dst, size));
+   case AF_INET6:
+-    return (inet_net_pton_ipv6(src, dst, size));
++    return (ares_inet_net_pton_ipv6(src, dst, size));
+   default:
+     SET_ERRNO(EAFNOSUPPORT);
+     return (-1);
+   }
+ }
+ 
+-#endif /* HAVE_INET_NET_PTON */
+-
+-#ifndef HAVE_INET_PTON
+ int ares_inet_pton(int af, const char *src, void *dst)
+ {
+   int result;
+@@ -434,11 +453,3 @@ int ares_inet_pton(int af, const char *src, void *dst)
+     return 0;
+   return (result > -1 ? 1 : -1);
+ }
+-#else /* HAVE_INET_PTON */
+-int ares_inet_pton(int af, const char *src, void *dst)
+-{
+-  /* just relay this to the underlying function */
+-  return inet_pton(af, src, dst);
+-}
+-
+-#endif
+diff --git a/test/ares-test-internal.cc b/test/ares-test-internal.cc
+index 1cb7e427dcdc..40cc82b86ec2 100644
+--- a/test/ares-test-internal.cc
++++ b/test/ares-test-internal.cc
+@@ -123,6 +123,7 @@ TEST_F(LibraryTest, InetPtoN) {
+   EXPECT_EQ(0, ares_inet_net_pton(AF_INET6, "12:34::ff/0", &a6, sizeof(a6)));
+   EXPECT_EQ(16 * 8, ares_inet_net_pton(AF_INET6, "12:34::ffff:0.2", &a6, sizeof(a6)));
+   EXPECT_EQ(16 * 8, ares_inet_net_pton(AF_INET6, "1234:1234:1234:1234:1234:1234:1234:1234", &a6, sizeof(a6)));
++  EXPECT_EQ(2, ares_inet_net_pton(AF_INET6, "0::00:00:00/2", &a6, sizeof(a6)));
+ 
+   // Various malformed versions
+   EXPECT_EQ(-1, ares_inet_net_pton(AF_INET, "", &a4, sizeof(a4)));
+@@ -160,11 +161,9 @@ TEST_F(LibraryTest, InetPtoN) {
+   EXPECT_EQ(-1, ares_inet_net_pton(AF_INET6, ":1234:1234:1234:1234:1234:1234:1234:1234", &a6, sizeof(a6)));
+   EXPECT_EQ(-1, ares_inet_net_pton(AF_INET6, ":1234:1234:1234:1234:1234:1234:1234:1234:", &a6, sizeof(a6)));
+   EXPECT_EQ(-1, ares_inet_net_pton(AF_INET6, "1234:1234:1234:1234:1234:1234:1234:1234:5678", &a6, sizeof(a6)));
+-  // TODO(drysdale): check whether the next two tests should give -1.
+-  EXPECT_EQ(0, ares_inet_net_pton(AF_INET6, "1234:1234:1234:1234:1234:1234:1234:1234:5678:5678", &a6, sizeof(a6)));
+-  EXPECT_EQ(0, ares_inet_net_pton(AF_INET6, "1234:1234:1234:1234:1234:1234:1234:1234:5678:5678:5678", &a6, sizeof(a6)));
++  EXPECT_EQ(-1, ares_inet_net_pton(AF_INET6, "1234:1234:1234:1234:1234:1234:1234:1234:5678:5678", &a6, sizeof(a6)));
++  EXPECT_EQ(-1, ares_inet_net_pton(AF_INET6, "1234:1234:1234:1234:1234:1234:1234:1234:5678:5678:5678", &a6, sizeof(a6)));
+   EXPECT_EQ(-1, ares_inet_net_pton(AF_INET6, "12:34::ffff:257.2.3.4", &a6, sizeof(a6)));
+-  EXPECT_EQ(-1, ares_inet_net_pton(AF_INET6, "12:34::ffff:002.2.3.4", &a6, sizeof(a6)));
+   EXPECT_EQ(-1, ares_inet_net_pton(AF_INET6, "12:34::ffff:1.2.3.4.5.6", &a6, sizeof(a6)));
+   EXPECT_EQ(-1, ares_inet_net_pton(AF_INET6, "12:34::ffff:1.2.3.4.5", &a6, sizeof(a6)));
+   EXPECT_EQ(-1, ares_inet_net_pton(AF_INET6, "12:34::ffff:1.2.3.z", &a6, sizeof(a6)));
+-- 
+2.40.1
+
diff -Nru c-ares-1.18.1/debian/patches/CVE-2023-32067.diff c-ares-1.18.1/debian/patches/CVE-2023-32067.diff
--- c-ares-1.18.1/debian/patches/CVE-2023-32067.diff	1970-01-01 01:00:00.000000000 +0100
+++ c-ares-1.18.1/debian/patches/CVE-2023-32067.diff	2023-05-23 14:34:52.000000000 +0200
@@ -0,0 +1,83 @@
+From: Brad House <brad@brad-house.com>
+Date: Mon, 22 May 2023 06:51:49 -0400
+Subject: Merge pull request from GHSA-9g78-jv2r-p7vc
+Origin: https://github.com/c-ares/c-ares/commit/b9b8413cfdb70a3f99e1573333b23052d57ec1ae
+Bug-Debian-Security: https://security-tracker.debian.org/tracker/CVE-2023-32067
+
+---
+ src/lib/ares_process.c | 41 +++++++++++++++++++++++++----------------
+ 1 file changed, 25 insertions(+), 16 deletions(-)
+
+diff --git a/src/lib/ares_process.c b/src/lib/ares_process.c
+index bf0cde464641..6cac0a99fdf9 100644
+--- a/src/lib/ares_process.c
++++ b/src/lib/ares_process.c
+@@ -470,7 +470,7 @@ static void read_udp_packets(ares_channel channel, fd_set *read_fds,
+ {
+   struct server_state *server;
+   int i;
+-  ares_ssize_t count;
++  ares_ssize_t read_len;
+   unsigned char buf[MAXENDSSZ + 1];
+ #ifdef HAVE_RECVFROM
+   ares_socklen_t fromlen;
+@@ -513,32 +513,41 @@ static void read_udp_packets(ares_channel channel, fd_set *read_fds,
+       /* To reduce event loop overhead, read and process as many
+        * packets as we can. */
+       do {
+-        if (server->udp_socket == ARES_SOCKET_BAD)
+-          count = 0;
+-
+-        else {
+-          if (server->addr.family == AF_INET)
++        if (server->udp_socket == ARES_SOCKET_BAD) {
++          read_len = -1;
++        } else {
++          if (server->addr.family == AF_INET) {
+             fromlen = sizeof(from.sa4);
+-          else
++          } else {
+             fromlen = sizeof(from.sa6);
+-          count = socket_recvfrom(channel, server->udp_socket, (void *)buf,
+-                                  sizeof(buf), 0, &from.sa, &fromlen);
++          }
++          read_len = socket_recvfrom(channel, server->udp_socket, (void *)buf,
++                                     sizeof(buf), 0, &from.sa, &fromlen);
+         }
+ 
+-        if (count == -1 && try_again(SOCKERRNO))
++        if (read_len == 0) {
++          /* UDP is connectionless, so result code of 0 is a 0-length UDP
++           * packet, and not an indication the connection is closed like on
++           * tcp */
+           continue;
+-        else if (count <= 0)
++        } else if (read_len < 0) {
++          if (try_again(SOCKERRNO))
++            continue;
++
+           handle_error(channel, i, now);
++
+ #ifdef HAVE_RECVFROM
+-        else if (!same_address(&from.sa, &server->addr))
++        } else if (!same_address(&from.sa, &server->addr)) {
+           /* The address the response comes from does not match the address we
+            * sent the request to. Someone may be attempting to perform a cache
+            * poisoning attack. */
+-          break;
++          continue;
+ #endif
+-        else
+-          process_answer(channel, buf, (int)count, i, 0, now);
+-       } while (count > 0);
++
++        } else {
++          process_answer(channel, buf, (int)read_len, i, 0, now);
++        }
++      } while (read_len >= 0);
+     }
+ }
+ 
+-- 
+2.40.1
+
diff -Nru c-ares-1.18.1/debian/patches/series c-ares-1.18.1/debian/patches/series
--- c-ares-1.18.1/debian/patches/series	2023-02-17 23:24:42.000000000 +0100
+++ c-ares-1.18.1/debian/patches/series	2023-05-23 14:34:52.000000000 +0200
@@ -1,2 +1,4 @@
 disable-cflags-rewrite.diff
 CVE-2022-4904.diff
+CVE-2023-31130.diff
+CVE-2023-32067.diff

Reply to: