Hi all, On Sat, 16 Nov 2019 11:57:19 +0100 Salvatore Bonaccorso <car...@debian.org> wrote: > Source: tnef > Version: 1.4.12-1.2 > Severity: grave > Tags: security upstream > Justification: user security hole > Forwarded: https://github.com/verdammelt/tnef/pull/40 > > Hi, > > The following vulnerability was published for tnef. > > CVE-2019-18849[0]: > | 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.
Whilst doing my LTS work, I fixed this for Jessie. Therefore, attaching a patch to fix this for Stretch, Buster, Bullseye, and Sid (since they're all at same version). Thus also CCing the Security team (if that's okay, we could perhaps upload the fix at the earliest :)). Hope this helps and gets this fixed asap :) Best, Utkarsh
Description: This patch fixes CVE-2019-18849. Fix strdup() on possibly unterminated string. Author: Paul Dreik Author: Utkarsh Gupta <guptautkarsh2...@gmail.com> Origin: https://github.com/verdammelt/tnef/pull/40 Bug-Debian: https://bugs.debian.org/944851 Last-Update: 2019-11-25 --- 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);
signature.asc
Description: OpenPGP digital signature
