File CVE-2020-8608-qemut-tcp_emu-fix-unsafe-snprintf-usages.patch of Package xen.25143
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.7.6-testing/tools/qemu-xen-traditional-dir-remote/slirp/tcp_subr.c
===================================================================
--- xen-4.7.6-testing.orig/tools/qemu-xen-traditional-dir-remote/slirp/tcp_subr.c
+++ xen-4.7.6-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);
