File CVE-2020-8608-qemut-tcp_emu-fix-unsafe-snprintf... of Package xen

Overview Repositories Revisions Requests Users Attributes Meta

File CVE-2020-8608-qemut-tcp_emu-fix-unsafe-snprintf-usages.patch of Package xen (Revision 2b74dd3853d0d0fb10f81949032d6b46)

Currently displaying revision 2b74dd3853d0d0fb10f81949032d6b46 , Show latest

Subject: util: add slirp_fmt() helpers
From: Marc-André Lureau marcandre.lureau@redhat.com Mon Jan 27 10:24:09 2020 +0100
Date: Mon Jan 27 10:28:43 2020 +0100:
Git: 30648c03b27fb8d9611b723184216cd3174b6775

Various calls to snprintf() in libslirp assume that snprintf() returns
"only" the number of bytes written (excluding terminating NUL).

https://pubs.opengroup.org/onlinepubs/9699919799/functions/snprintf.html#tag_16_159_04

"Upon successful completion, the snprintf() function shall return the
number of bytes that would be written to s had n been sufficiently
large excluding the terminating null byte."

Introduce slirp_fmt() that handles several pathological cases the
way libslirp usually expect:

- treat error as fatal (instead of silently returning -1)

- fmt0() will always \0 end

- return the number of bytes actually written (instead of what would
have been written, which would usually result in OOB later), including
the ending \0 for fmt0()

- warn if truncation happened (instead of ignoring)

Other less common cases can still be handled with strcpy/snprintf() etc.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
Message-Id: <20200127092414.169796-2-marcandre.lureau@redhat.com>

Subject: tcp_emu: fix unsafe snprintf() usages
From: Marc-André Lureau marcandre.lureau@redhat.com Mon Jan 27 10:24:14 2020 +0100
Date: Mon Jan 27 10:28:43 2020 +0100:
Git: 68ccb8021a838066f0951d4b2817eb6b6f10a843

Various calls to snprintf() assume that snprintf() returns "only" the
number of bytes written (excluding terminating NUL).

https://pubs.opengroup.org/onlinepubs/9699919799/functions/snprintf.html#tag_16_159_04

"Upon successful completion, the snprintf() function shall return the
number of bytes that would be written to s had n been sufficiently
large excluding the terminating null byte."

Before patch ce131029, if there isn't enough room in "m_data" for the
"DCC ..." message, we overflow "m_data".

After the patch, if there isn't enough room for the same, we don't
overflow "m_data", but we set "m_len" out-of-bounds. The next time an
access is bounded by "m_len", we'll have a buffer overflow then.

Use slirp_fmt*() to fix potential OOB memory access.

Reported-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
Message-Id: <20200127092414.169796-7-marcandre.lureau@redhat.com>

Index: xen-4.9.4-testing/tools/qemu-xen-traditional-dir-remote/slirp/tcp_subr.c
===================================================================
--- xen-4.9.4-testing.orig/tools/qemu-xen-traditional-dir-remote/slirp/tcp_subr.c
+++ xen-4.9.4-testing/tools/qemu-xen-traditional-dir-remote/slirp/tcp_subr.c
@@ -595,6 +595,68 @@ tcp_tos(so)
 int do_echo = -1;
 #endif
 
+static int slirp_vsnprintf(char *str, size_t size,
+                           const char *format, va_list args)
+{
+    int rv = vsnprintf(str, size, format, args);
+
+    if (rv < 0) {
+        g_error("vsnprintf() failed: %s", g_strerror(errno));
+    }
+
+    return rv;
+}
+
+/*
+ * A snprintf()-like function that:
+ * - returns the number of bytes written (excluding optional \0-ending)
+ * - dies on error
+ * - warn on truncation
+ */
+static int slirp_fmt(char *str, size_t size, const char *format, ...)
+{
+    va_list args;
+    int rv;
+
+    va_start(args, format);
+    rv = slirp_vsnprintf(str, size, format, args);
+    va_end(args);
+
+    if (rv > size) {
+        g_critical("vsnprintf() truncation");
+    }
+
+    return MIN(rv, size);
+}
+
+/*
+ * A snprintf()-like function that:
+ * - always \0-end (unless size == 0)
+ * - returns the number of bytes actually written, including \0 ending
+ * - dies on error
+ * - warn on truncation
+ */
+static int slirp_fmt0(char *str, size_t size, const char *format, ...)
+{
+    va_list args;
+    int rv;
+
+    va_start(args, format);
+    rv = slirp_vsnprintf(str, size, format, args);
+    va_end(args);
+
+    if (rv >= size) {
+        g_critical("vsnprintf() truncation");
+        if (size > 0)
+            str[size - 1] = '\0';
+        rv = size;
+    } else {
+        rv += 1; /* include \0 */
+    }
+
+    return rv;
+}
+
 /*
  * Emulate programs that try and connect to us
  * This includes ftp (the data connection is
@@ -673,7 +735,7 @@ tcp_emu(so, m)
 							break;
 						}
 					}
-                                        so_rcv->sb_cc = snprintf(so_rcv->sb_data,
+                                        so_rcv->sb_cc = slirp_fmt(so_rcv->sb_data,
                                                                  so_rcv->sb_datalen,
                                                                  "%d,%d\r\n", n1, n2);
 				        so_rcv->sb_rptr = so_rcv->sb_data;
@@ -1010,7 +1072,7 @@ do_prompt:
 			n4 =  (laddr & 0xff);
 
 			m->m_len = bptr - m->m_data; /* Adjust length */
-                        m->m_len += snprintf(bptr, m->m_hdr.mh_size - m->m_len,
+                        m->m_len += slirp_fmt(bptr, m->m_hdr.mh_size - m->m_len,
                                              "ORT %d,%d,%d,%d,%d,%d\r\n%s",
                                              n1, n2, n3, n4, n5, n6, x==7?buff:"");
 			return 1;
@@ -1042,7 +1104,7 @@ do_prompt:
 			n4 =  (laddr & 0xff);
 
 			m->m_len = bptr - m->m_data; /* Adjust length */
-			m->m_len += snprintf(bptr, m->m_hdr.mh_size - m->m_len,
+			m->m_len += slirp_fmt(bptr, m->m_hdr.mh_size - m->m_len,
                                              "27 Entering Passive Mode (%d,%d,%d,%d,%d,%d)\r\n%s",
                                              n1, n2, n3, n4, n5, n6, x==7?buff:"");
 
@@ -1067,7 +1129,7 @@ do_prompt:
 		}
 		if (m->m_data[m->m_len-1] == '\0' && lport != 0 &&
 		    (so = solisten(0, so->so_laddr.s_addr, htons(lport), SS_FACCEPTONCE)) != NULL)
-                    m->m_len = snprintf(m->m_data, m->m_hdr.mh_size, "%d",
+                    m->m_len = slirp_fmt0(m->m_data, m->m_hdr.mh_size, "%d",
                                         ntohs(so->so_fport)) + 1;
 		return 1;
 
@@ -1085,7 +1147,7 @@ do_prompt:
 				return 1;
 
 			m->m_len = bptr - m->m_data; /* Adjust length */
-                        m->m_len += snprintf(bptr, m->m_hdr.mh_size,
+                        m->m_len += slirp_fmt(bptr, m->m_hdr.mh_size,
                                              "DCC CHAT chat %lu %u%c\n",
                                              (unsigned long)ntohl(so->so_faddr.s_addr),
                                              ntohs(so->so_fport), 1);
@@ -1094,7 +1156,7 @@ do_prompt:
 				return 1;
 
 			m->m_len = bptr - m->m_data; /* Adjust length */
-                        m->m_len += snprintf(bptr, m->m_hdr.mh_size,
+                        m->m_len += slirp_fmt(bptr, m->m_hdr.mh_size,
                                              "DCC SEND %s %lu %u %u%c\n", buff,
                                              (unsigned long)ntohl(so->so_faddr.s_addr),
                                              ntohs(so->so_fport), n1, 1);
@@ -1103,7 +1165,7 @@ do_prompt:
 				return 1;
 
 			m->m_len = bptr - m->m_data; /* Adjust length */
-                        m->m_len += snprintf(bptr, m->m_hdr.mh_size,
+                        m->m_len += slirp_fmt(bptr, m->m_hdr.mh_size,
                                              "DCC MOVE %s %lu %u %u%c\n", buff,
                                              (unsigned long)ntohl(so->so_faddr.s_addr),
                                              ntohs(so->so_fport), n1, 1);
@@ -1299,7 +1361,7 @@ tcp_ctl(so)
 
 		/* FALLTHROUGH */
 	case CTL_ALIAS:
-          sb->sb_cc = snprintf(sb->sb_wptr, sb->sb_datalen - (sb->sb_wptr - sb->sb_data),
+          sb->sb_cc = slirp_fmt(sb->sb_wptr, sb->sb_datalen - (sb->sb_wptr - sb->sb_data),
                                "Error: No application configured.\r\n");
 	  sb->sb_wptr += sb->sb_cc;
 	  return(0);

Places

File CVE-2020-8608-qemut-tcp_emu-fix-unsafe-snprintf-usages.patch of Package xen (Revision 2b74dd3853d0d0fb10f81949032d6b46)

Places