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

Bug#988504: marked as done (buster-pu: package tnef/1.4.12-1.2)



Your message dated Sat, 15 May 2021 10:58:39 +0200
with message-id <[🔎] YJ+NPy4hPKNd74Ui@eldamar.lan>
and subject line Re: Bug#988504: buster-pu: package tnef/1.4.12-1.2
has caused the Debian Bug report #988504,
regarding buster-pu: package tnef/1.4.12-1.2
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact owner@bugs.debian.org
immediately.)


-- 
988504: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=988504
Debian Bug Tracking System
Contact owner@bugs.debian.org with problems
--- Begin Message ---
Package: release.debian.org
Severity: normal
Tags: buster
User: release.debian.org@packages.debian.org
Usertags: pu
X-Debbugs-Cc: haavard_aasen@yahoo.no

Added patch to fix CVE-2019-18849, bug #944851. The patch is identical
to that applied in jessie, but I also controlled it against the upstream
commit, to make sure nothing had changed and everything is included.

[ Reason ]
Fix: CVE-2019-18849 and bug: #944851
In tnef before 1.4.18, an attacker may be able to write to the victim's
.ssh/authorized_keys file via an e-mail message with a crafted
winmail.dat application/ms-tnef attachment, because of a heap-based
buffer over-read involving strdup.

[ Impact ]

[ Tests ]
None, but the original patch is from upstream. This exact patch has also been
included in jessie since late 2019

[ Risks ]
I consider the risk to be small since the code has been implemented by
upstream and has been included in jessie.

[ 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 (old)stable
  [x] the issue is verified as fixed in unstable

[ Changes ]
The changes is to prevent the possibility of not terminating strings with
strdup()


Regards,
Håvard
diff -Nru tnef-1.4.12/debian/changelog tnef-1.4.12/debian/changelog
--- tnef-1.4.12/debian/changelog	2017-05-29 15:03:02.000000000 +0200
+++ tnef-1.4.12/debian/changelog	2021-05-14 11:14:22.000000000 +0200
@@ -1,3 +1,15 @@
+tnef (1.4.12-1.2+deb10u1) buster; urgency=medium
+
+  * Non-maintainer upload.
+  * CVE-2019-18849
+    In tnef before 1.4.18, an attacker may be able to write to the
+    victim's .ssh/authorized_keys file via an e-mail message with a
+    crafted winmail.dat application/ms-tnef attachment, because of
+    a heap-based buffer over-read involving strdup.
+    Closes: #944851
+
+ -- Håvard Flaget Aasen <haavard_aasen@yahoo.no>  Fri, 14 May 2021 11:14:22 +0200
+
 tnef (1.4.12-1.2) unstable; urgency=medium
 
   * Non-maintainer upload by the Wheezy LTS Team. (Closes: #862442)
diff -Nru tnef-1.4.12/debian/patches/CVE-2019-18849.patch tnef-1.4.12/debian/patches/CVE-2019-18849.patch
--- tnef-1.4.12/debian/patches/CVE-2019-18849.patch	1970-01-01 01:00:00.000000000 +0100
+++ tnef-1.4.12/debian/patches/CVE-2019-18849.patch	2021-05-14 11:11:07.000000000 +0200
@@ -0,0 +1,146 @@
+Description: This patch fixes CVE-2019-18849.
+ Fix strdup() on possibly unterminated string.
+Author: Paul Dreik
+Author: Utkarsh Gupta <guptautkarsh2102@gmail.com>
+Origin: https://github.com/verdammelt/tnef/pull/40
+Bug-Debian: https://bugs.debian.org/944851
+Reviewed-by: Håvard Flaget Aasen <haavard_aasen@yahoo.no>
+Last-Update: 2021-05-14
+
+--- a/src/alloc.c
++++ b/src/alloc.c
+@@ -72,13 +72,14 @@
+ 
+ /* attempts to malloc memory, if fails print error and call abort */
+ void*
+-xmalloc (size_t num, size_t size)
++xmalloc (size_t num, size_t size, size_t extra)
+ {
+     size_t res;
+     if (check_mul_overflow(num, size, &res))
+         abort();
+-
+-    void *ptr = malloc (res);
++    if (res + extra < res)
++        abort();
++    void *ptr = malloc (res + extra);
+     if (!ptr
+         && (size != 0))         /* some libc don't like size == 0 */
+     {
+@@ -90,41 +91,44 @@
+ 
+ /* Allocates memory but only up to a limit */
+ void*
+-checked_xmalloc (size_t num, size_t size)
++checked_xmalloc (size_t num, size_t size, size_t extra)
+ {
+     size_t res;
+     if (check_mul_overflow(num, size, &res))
+         abort();
+-
++    if (res + extra < res)
++        abort();
+     alloc_limit_assert ("checked_xmalloc", res);
+-    return xmalloc (num, size);
++    return xmalloc (num, size, extra);
+ }
+ 
+ /* xmallocs memory and clears it out */
+ void*
+-xcalloc (size_t num, size_t size)
++xcalloc (size_t num, size_t size, size_t extra)
+ {
+     size_t res;
+     if (check_mul_overflow(num, size, &res))
+         abort();
+ 
+     void *ptr;
+-    ptr = malloc(res);
++    if (res + extra < res)
++        abort();
++    ptr = malloc(res + extra);
+     if (ptr)
+     {
+-        memset (ptr, '\0', (res));
++        memset (ptr, '\0', (res + extra));
+     }
+     return ptr;
+ }
+ 
+ /* xcallocs memory but only up to a limit */
+ void*
+-checked_xcalloc (size_t num, size_t size)
++checked_xcalloc (size_t num, size_t size, size_t extra)
+ {
+     size_t res;
+     if (check_mul_overflow(num, size, &res))
+         abort();
+ 
+     alloc_limit_assert ("checked_xcalloc", (res));
+-    return xcalloc (num, size);
++    return xcalloc (num, size, extra);
+ }
+--- a/src/alloc.h
++++ b/src/alloc.h
+@@ -35,19 +35,23 @@
+ extern void set_alloc_limit (size_t size);
+ extern size_t get_alloc_limit();
+ extern void alloc_limit_assert (char *fn_name, size_t size);
+-extern void* checked_xmalloc (size_t num, size_t size);
+-extern void* xmalloc (size_t num, size_t size);
+-extern void* checked_xcalloc (size_t num, size_t size);
+-extern void* xcalloc (size_t num, size_t size);
++extern void* checked_xmalloc (size_t num, size_t size, size_t extra);
++extern void* xmalloc (size_t num, size_t size, size_t extra);
++extern void* checked_xcalloc (size_t num, size_t size, size_t extra);
++extern void* xcalloc (size_t num, size_t size, size_t extra);
+ 
+ #define XMALLOC(_type,_num)			                \
+-        ((_type*)xmalloc((_num), sizeof(_type)))
++  ((_type*)xmalloc((_num), sizeof(_type), 0))
+ #define XCALLOC(_type,_num) 				        \
+-        ((_type*)xcalloc((_num), sizeof (_type)))
++  ((_type*)xcalloc((_num), sizeof (_type), 0))
+ #define CHECKED_XMALLOC(_type,_num) 			        \
+-        ((_type*)checked_xmalloc((_num),sizeof(_type)))
+-#define CHECKED_XCALLOC(_type,_num) 			        \
+-        ((_type*)checked_xcalloc((_num),sizeof(_type)))
++  ((_type*)checked_xmalloc((_num),sizeof(_type),0))
++#define CHECKED_XMALLOC_ADDNULL(_type,_num)                     \
++  ((_type*)checked_xmalloc((_num),sizeof(_type),1))
++#define CHECKED_XCALLOC(_type,_num)         \
++  ((_type*)checked_xcalloc((_num),sizeof(_type),0))
++#define CHECKED_XCALLOC_ADDNULL(_type,_num)     \
++  ((_type*)checked_xcalloc((_num),sizeof(_type),1))
+ #define XFREE(_ptr)						\
+         do { if (_ptr) { free (_ptr); _ptr = 0; } } while (0)
+ 
+--- a/src/attr.c
++++ b/src/attr.c
+@@ -244,7 +244,11 @@
+     attr->type = (type_and_name >> 16);
+     attr->name = ((type_and_name << 16) >> 16);
+     attr->len = geti32(in);
+-    attr->buf = CHECKED_XCALLOC (unsigned char, attr->len);
++    /* Allocate an extra byte for the null terminator,
++       in case the input lacks it,
++       this avoids strdup() being invoked on possibly non-terminated
++       input later (file.c, file_add_attr()). */
++    attr->buf = CHECKED_XCALLOC_ADDNULL(unsigned char, attr->len);
+     
+     (void)getbuf(in, attr->buf, attr->len);
+     
+--- a/src/mapi_attr.c
++++ b/src/mapi_attr.c
+@@ -314,8 +314,10 @@
+ 		}
+ 		else
+ 		{
+-		    v->data.buf = CHECKED_XMALLOC(unsigned char, v->len);
++            /* add space for a null terminator, in case of evil input */
++            v->data.buf = CHECKED_XMALLOC_ADDNULL(unsigned char, v->len);
+ 		    memmove (v->data.buf, buf+idx, v->len);
++            v->data.buf[v->len] = '\0';
+ 		}
+ 
+ 		idx += pad_to_4byte(v->len);
diff -Nru tnef-1.4.12/debian/patches/series tnef-1.4.12/debian/patches/series
--- tnef-1.4.12/debian/patches/series	2017-05-29 15:03:02.000000000 +0200
+++ tnef-1.4.12/debian/patches/series	2021-05-14 10:54:30.000000000 +0200
@@ -4,3 +4,4 @@
 fix-regression-1.patch
 fix-regression-2.patch
 CVE-2017-8911.patch
+CVE-2019-18849.patch

--- End Message ---
--- Begin Message ---
On Fri, May 14, 2021 at 12:11:59PM +0200, Håvard Flaget Aasen wrote:
> Package: release.debian.org
> Severity: normal
> Tags: buster
> User: release.debian.org@packages.debian.org
> Usertags: pu
> X-Debbugs-Cc: haavard_aasen@yahoo.no
> 
> Added patch to fix CVE-2019-18849, bug #944851. The patch is identical
> to that applied in jessie, but I also controlled it against the upstream
> commit, to make sure nothing had changed and everything is included.
> 
> [ Reason ]
> Fix: CVE-2019-18849 and bug: #944851
> In tnef before 1.4.18, an attacker may be able to write to the victim's
> .ssh/authorized_keys file via an e-mail message with a crafted
> winmail.dat application/ms-tnef attachment, because of a heap-based
> buffer over-read involving strdup.
> 
> [ Impact ]
> 
> [ Tests ]
> None, but the original patch is from upstream. This exact patch has also been
> included in jessie since late 2019
> 
> [ Risks ]
> I consider the risk to be small since the code has been implemented by
> upstream and has been included in jessie.
> 
> [ 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 (old)stable
>   [x] the issue is verified as fixed in unstable
> 
> [ Changes ]
> The changes is to prevent the possibility of not terminating strings with
> strdup()

Thorsten Alteholz already proposed an update for tnef in #987246,
which needs an ack yet from the release team.

Regards,
Salvatore

--- End Message ---

Reply to: