Re: slirp / CVE-2020-7039 / CVE-2020-8608
Roberto C. Sánchez <roberto@debian.org> writes:
> On Wed, Aug 12, 2020 at 08:55:43AM +1000, Brian May wrote:
>> I am seriously thinking that slirp from unstable should be ported as is
>> from sid to buster and stretch. This is not a new upstream version, it
>> has bug fixes and security updates only. Probably the same changes I
>> would have to make myself in fact. Such as replacing sprintf calls with
>> snprintf calls for example.
>>
>> This would fix CVE-2020-7039 and provide the prerequisite to fixing
>> CVE-2020-8608.
>>
>> Only thing, I am not sure what to do with the versioning:
>>
>> stretch 1:1.0.17-8
>> buster 1:1.0.17-8
>> sid 1:1.0.17-10
>>
>> In fact, because stretch and buster has the same version, does this mean
>> I can't make any security uploads to stretch?
>>
>> On the other hand the security team has marked both these as no-DSA, in
>> buster meaning maybe I should do the same thing too?
>
> I would ask the Security Team if they are open to considering taking
> 1:1.0.17-10 into buster. The version would be 1:1.0.17-10~deb10u1. If
> they agree, then you could subsequently upload to stretch with version
> 1:1.0.17-10~deb9u1. If they are not open to considering it, then it
> seems that the only viable course of action is the mark them no-dsa.
I think perhaps the appropriate course of action would be to fix it in
sid, then back port to the other distributions.
But fixing sid requires fixing #778124 first. OK, that is a simple fix,
but surprised this ever worked.
Attached is my WIP patch. It doesn't quite compile just yet, because it
refers to functions like g_critical, g_error and g_strerror which will
probably need to be replaced by something else. It also doesn't yet fix
the IDENT service.
I also notice a number of untouched calls to "sprintf" which make me
nervous, and perhaps every call to "snprintf" should be replaced with a
call to slirp_fmt0 (because snprintf doesn't guarantee the result will
be null terminated). Including the IDENT service, which the patch
changes, but the code is a bit different, so not easy to apply as is.
No point doing an incomplete fix I think.
I suspect every instance of sprintf and snprintf needs to be replaced
with the slirp_fmt0 call. Existing sprintf calls will also need an extra
argument.
Maybe a better approach would be to switch to the latest upstream
version and use that everywhere?
--
Brian May <bam@debian.org>
diff -Nru slirp-1.0.17/debian/changelog slirp-1.0.17/debian/changelog
--- slirp-1.0.17/debian/changelog 2020-01-25 06:12:54.000000000 +1100
+++ slirp-1.0.17/debian/changelog 2020-08-13 09:04:48.000000000 +1000
@@ -1,3 +1,11 @@
+slirp (1:1.0.17-10.1) unstable; urgency=medium
+
+ * Non-maintainer upload.
+ * Fix FTBS by deduplicating symbols (Closes: #778124).
+ * Fix for CVE-2020-8608.
+
+ -- Brian May <bam@debian.org> Thu, 13 Aug 2020 09:04:48 +1000
+
slirp (1:1.0.17-10) unstable; urgency=high
* Fix for CVE-2020-7039 (Closes: #949085)
diff -Nru slirp-1.0.17/debian/patches/015_fix_duplicate_definitions.patch slirp-1.0.17/debian/patches/015_fix_duplicate_definitions.patch
--- slirp-1.0.17/debian/patches/015_fix_duplicate_definitions.patch 1970-01-01 10:00:00.000000000 +1000
+++ slirp-1.0.17/debian/patches/015_fix_duplicate_definitions.patch 2020-08-13 08:16:48.000000000 +1000
@@ -0,0 +1,23 @@
+--- a/src/options.c
++++ b/src/options.c
+@@ -74,10 +74,6 @@
+ * Read the config file
+ */
+
+-int (*lprint_print) _P((void *, const char *format, va_list));
+-char *lprint_ptr, *lprint_ptr2, **lprint_arg;
+-struct sbuf *lprint_sb;
+-
+ int cfg_unit;
+ int ctl_password_ok;
+ char *ctl_password;
+--- a/src/misc.c
++++ b/src/misc.c
+@@ -593,6 +593,7 @@
+
+ int (*lprint_print) _P((void *, const char *, va_list));
+ char *lprint_ptr, *lprint_ptr2, **lprint_arg;
++struct sbuf *lprint_sb;
+
+ void
+ #ifdef __STDC__
diff -Nru slirp-1.0.17/debian/patches/CVE-2020-8608.patch slirp-1.0.17/debian/patches/CVE-2020-8608.patch
--- slirp-1.0.17/debian/patches/CVE-2020-8608.patch 1970-01-01 10:00:00.000000000 +1000
+++ slirp-1.0.17/debian/patches/CVE-2020-8608.patch 2020-08-13 08:42:37.000000000 +1000
@@ -0,0 +1,133 @@
+--- a/src/tcp_subr.c
++++ b/src/tcp_subr.c
+@@ -1015,7 +1015,7 @@
+ n4 = (laddr & 0xff);
+
+ m->m_len = bptr - m->m_data; /* Adjust length */
+- m->m_len += snprintf(bptr, M_FREEROOM(m), "ORT %d,%d,%d,%d,%d,%d\r\n%s",
++ m->m_len += slirp_fmt(bptr, M_FREEROOM(m), "ORT %d,%d,%d,%d,%d,%d\r\n%s",
+ n1, n2, n3, n4, n5, n6, x==7?buff:"");
+ return 1;
+ } else if ((bptr = (char *)strstr(m->m_data, "27 Entering")) != NULL) {
+@@ -1046,7 +1046,7 @@
+ n4 = (laddr & 0xff);
+
+ m->m_len = bptr - m->m_data; /* Adjust length */
+- m->m_len += snprintf(bptr, M_FREEROOM(m),
++ m->m_len += slirp_fmt(bptr, M_FREEROOM(m),
+ "27 Entering Passive Mode (%d,%d,%d,%d,%d,%d)\r\n%s",
+ n1, n2, n3, n4, n5, n6, x==7?buff:"");
+
+@@ -1071,7 +1071,7 @@
+ }
+ 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_ROOM(m), "%d", ntohs(so->so_fport)) + 1;
++ m->m_len = slirp_fmt0(m->m_data, M_ROOM(m), "%d", ntohs(so->so_fport));
+ return 1;
+
+ case EMU_IRC:
+@@ -1088,7 +1088,7 @@
+ return 1;
+
+ m->m_len = bptr - m->m_data; /* Adjust length */
+- m->m_len += snprintf(bptr, M_FREEROOM(m), "DCC CHAT chat %lu %u%c\n",
++ m->m_len += slirp_fmt(bptr, M_FREEROOM(m), "DCC CHAT chat %lu %u%c\n",
+ (unsigned long)ntohl(so->so_faddr.s_addr),
+ ntohs(so->so_fport), 1);
+ } else if (sscanf(bptr, "DCC SEND %256s %u %u %u", buff, &laddr, &lport, &n1) == 4) {
+@@ -1096,7 +1096,7 @@
+ return 1;
+
+ m->m_len = bptr - m->m_data; /* Adjust length */
+- m->m_len += snprintf(bptr, M_FREEROOM(m), "DCC SEND %s %lu %u %u%c\n",
++ m->m_len += slirp_fmt(bptr, M_FREEROOM(m), "DCC SEND %s %lu %u %u%c\n",
+ buff, (unsigned long)ntohl(so->so_faddr.s_addr),
+ ntohs(so->so_fport), n1, 1);
+ } else if (sscanf(bptr, "DCC MOVE %256s %u %u %u", buff, &laddr, &lport, &n1) == 4) {
+@@ -1104,7 +1104,7 @@
+ return 1;
+
+ m->m_len = bptr - m->m_data; /* Adjust length */
+- m->m_len += snprintf(bptr, M_FREEROOM(m), "DCC MOVE %s %lu %u %u%c\n",
++ m->m_len += slirp_fmt(bptr, M_FREEROOM(m), "DCC MOVE %s %lu %u %u%c\n",
+ buff, (unsigned long)ntohl(so->so_faddr.s_addr),
+ ntohs(so->so_fport), n1, 1);
+ }
+--- a/src/misc.c
++++ b/src/misc.c
+@@ -1016,8 +1016,64 @@
+ }
+
+
++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
++ */
++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
++ */
++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;
++}
+--- a/src/misc.h
++++ b/src/misc.h
+@@ -67,4 +67,7 @@
+
+ extern int x_port, x_server, x_display;
+
++int slirp_fmt(char *str, size_t size, const char *format, ...);
++int slirp_fmt0(char *str, size_t size, const char *format, ...);
++
+ #endif
diff -Nru slirp-1.0.17/debian/patches/series slirp-1.0.17/debian/patches/series
--- slirp-1.0.17/debian/patches/series 2020-01-24 22:02:28.000000000 +1100
+++ slirp-1.0.17/debian/patches/series 2020-08-13 08:21:22.000000000 +1000
@@ -12,3 +12,5 @@
012_ipq64.patch
013_hurd.patch
014_CVE-2020-7039.patch
+015_fix_duplicate_definitions.patch
+CVE-2020-8608.patch
Reply to: