Re: slirp / CVE-2020-7039 / CVE-2020-8608

2020-08-12 Thread Brian May
Roberto C. Sánchez  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 
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.0 +1100
+++ slirp-1.0.17/debian/changelog	2020-08-13 09:04:48.0 +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   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.0 +1000
+++ slirp-1.0.17/debian/patches/015_fix_duplicate_definitions.patch	2020-08-13 08:16:48.0 +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.0 +1000
+++ slirp-1.0.17/debian/patches/CVE-2020-8608.patch	2020-08-13 08:42:37.0 +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_la

Re: slirp / CVE-2020-7039 / CVE-2020-8608

2020-08-12 Thread Emilio Pozuelo Monfort
On 12/08/2020 01:04, Roberto C. Sánchez wrote:
> 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.

Even if it's no-dsa, it can still be updated in buster via 
stable-proposed-updates.

Cheers,
Emilio



Re: slirp / CVE-2020-7039 / CVE-2020-8608

2020-08-11 Thread Roberto C . Sánchez
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.

Regards,

-Roberto

-- 
Roberto C. Sánchez



slirp / CVE-2020-7039 / CVE-2020-8608

2020-08-11 Thread Brian May
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?
-- 
Brian May 
https://linuxpenguins.xyz/brian/