Hello community, here is the log from the commit of package libX11 for openSUSE:Factory checked in at 2020-08-05 20:26:35 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ Comparing /work/SRC/openSUSE:Factory/libX11 (Old) and /work/SRC/openSUSE:Factory/.libX11.new.3592 (New) ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Package is "libX11" Wed Aug 5 20:26:35 2020 rev:27 rq:824353 version:1.6.9 Changes: -------- --- /work/SRC/openSUSE:Factory/libX11/libX11.changes 2019-10-24 23:01:07.107960559 +0200 +++ /work/SRC/openSUSE:Factory/.libX11.new.3592/libX11.changes 2020-08-05 20:27:05.483038834 +0200 @@ -1,0 +2,17 @@ +Tue Aug 4 16:33:45 CEST 2020 - [email protected] + +- U_006-Fix-size-calculation-in-_XimAttributeToValue.patch: + * Regression fix in previous XIM client head overflow fixes + (CVE-2020-14344, bsc#1174628) + +------------------------------------------------------------------- +Fri Jul 31 20:23:05 UTC 2020 - Stefan Dirsch <[email protected]> + +- U_001-ChangeTheData_lenParameterOf_XimAttributeToValueToCARD16.patch, + U_002-FixIntegerOverflowsIn_XimAttributeToValue.patch, + U_003-FixMoreUncheckedLengths.patch, + U_004-FixSignedLengthValuesIn_XimGetAttributeID.patch, + U_005-ZeroOutBuffersInFunctions.patch, + * XIM client heap overflows (CVE-2020-14344, bsc#1174628) + +------------------------------------------------------------------- New: ---- U_001-ChangeTheData_lenParameterOf_XimAttributeToValueToCARD16.patch U_002-FixIntegerOverflowsIn_XimAttributeToValue.patch U_003-FixMoreUncheckedLengths.patch U_004-FixSignedLengthValuesIn_XimGetAttributeID.patch U_005-ZeroOutBuffersInFunctions.patch U_006-Fix-size-calculation-in-_XimAttributeToValue.patch ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ Other differences: ------------------ ++++++ libX11.spec ++++++ --- /var/tmp/diff_new_pack.0pJO7q/_old 2020-08-05 20:27:07.947039614 +0200 +++ /var/tmp/diff_new_pack.0pJO7q/_new 2020-08-05 20:27:07.951039615 +0200 @@ -1,7 +1,7 @@ # # spec file for package libX11 # -# Copyright (c) 2019 SUSE LINUX GmbH, Nuernberg, Germany. +# Copyright (c) 2020 SUSE LLC # # All modifications and additions to the file contributed by third parties # remain the property of their copyright owners, unless otherwise agreed @@ -33,6 +33,12 @@ Patch1: p_xlib_skip_ext_env.diff # PATCH-FIX-UPSTREAM en-locales.diff fdo#48596 bnc#388711 -- Add missing data for more en locales Patch2: en-locales.diff +Patch21: U_001-ChangeTheData_lenParameterOf_XimAttributeToValueToCARD16.patch +Patch22: U_002-FixIntegerOverflowsIn_XimAttributeToValue.patch +Patch23: U_003-FixMoreUncheckedLengths.patch +Patch24: U_004-FixSignedLengthValuesIn_XimGetAttributeID.patch +Patch25: U_005-ZeroOutBuffersInFunctions.patch +Patch26: U_006-Fix-size-calculation-in-_XimAttributeToValue.patch BuildRequires: fdupes BuildRequires: libtool BuildRequires: pkgconfig @@ -133,7 +139,15 @@ test -f nls/ja.U90/XLC_LOCALE.pre && exit 1 test -f nls/ja.S90/XLC_LOCALE.pre && exit 1 -%autopatch -p0 +%patch0 +%patch1 +%patch2 +%patch21 -p1 +%patch22 -p1 +%patch23 -p1 +%patch24 -p1 +%patch25 -p1 +%patch26 -p1 %build %configure \ ++++++ U_001-ChangeTheData_lenParameterOf_XimAttributeToValueToCARD16.patch ++++++ It's coming from a length in the protocol (unsigned) and passed to functions that expect unsigned int parameters (_XCopyToArg() and memcpy()). Signed-off-by: Matthieu Herrb <[email protected]> Reviewed-by: Todd Carson <[email protected]> --- modules/im/ximcp/imRmAttr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Index: libX11-1.6.5/modules/im/ximcp/imRmAttr.c =================================================================== --- libX11-1.6.5.orig/modules/im/ximcp/imRmAttr.c +++ libX11-1.6.5/modules/im/ximcp/imRmAttr.c @@ -214,7 +214,7 @@ _XimAttributeToValue( Xic ic, XIMResourceList res, CARD16 *data, - INT16 data_len, + CARD16 data_len, XPointer value, BITMASK32 mode) { ++++++ U_002-FixIntegerOverflowsIn_XimAttributeToValue.patch ++++++ From: Todd Carson <[email protected]> Signed-off-by: Matthieu Herrb <[email protected]> Reviewed-by: Matthieu Herrb <[email protected]> --- modules/im/ximcp/imRmAttr.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/modules/im/ximcp/imRmAttr.c b/modules/im/ximcp/imRmAttr.c index d5d1939e..db3639de 100644 --- a/modules/im/ximcp/imRmAttr.c +++ b/modules/im/ximcp/imRmAttr.c @@ -29,6 +29,8 @@ PERFORMANCE OF THIS SOFTWARE. #ifdef HAVE_CONFIG_H #include <config.h> #endif +#include <limits.h> + #include "Xlibint.h" #include "Xlcint.h" #include "Ximint.h" @@ -250,18 +252,24 @@ _XimAttributeToValue( case XimType_XIMStyles: { - INT16 num = data[0]; + CARD16 num = data[0]; register CARD32 *style_list = (CARD32 *)&data[2]; XIMStyle *style; XIMStyles *rep; register int i; char *p; - int alloc_len; + unsigned int alloc_len; if (!(value)) return False; + if (num > (USHRT_MAX / sizeof(XIMStyle))) + return False; + if ((sizeof(num) + (num * sizeof(XIMStyle))) > data_len) + return False; alloc_len = sizeof(XIMStyles) + sizeof(XIMStyle) * num; + if (alloc_len < sizeof(XIMStyles)) + return False; if (!(p = Xmalloc(alloc_len))) return False; @@ -357,19 +365,25 @@ _XimAttributeToValue( case XimType_XIMHotKeyTriggers: { - INT32 num = *((CARD32 *)data); + CARD32 num = *((CARD32 *)data); register CARD32 *key_list = (CARD32 *)&data[2]; XIMHotKeyTrigger *key; XIMHotKeyTriggers *rep; register int i; char *p; - int alloc_len; + unsigned int alloc_len; if (!(value)) return False; + if (num > (UINT_MAX / sizeof(XIMHotKeyTrigger))) + return False; + if ((sizeof(num) + (num * sizeof(XIMHotKeyTrigger))) > data_len) + return False; alloc_len = sizeof(XIMHotKeyTriggers) + sizeof(XIMHotKeyTrigger) * num; + if (alloc_len < sizeof(XIMHotKeyTriggers)) + return False; if (!(p = Xmalloc(alloc_len))) return False; ++++++ U_003-FixMoreUncheckedLengths.patch ++++++ From: Todd Carson <[email protected]> Signed-off-by: Matthieu Herrb <[email protected]> Reviewed-by: Matthieu Herrb <[email protected]> --- modules/im/ximcp/imRmAttr.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/modules/im/ximcp/imRmAttr.c b/modules/im/ximcp/imRmAttr.c index db3639de..b7591a07 100644 --- a/modules/im/ximcp/imRmAttr.c +++ b/modules/im/ximcp/imRmAttr.c @@ -321,7 +321,7 @@ _XimAttributeToValue( case XimType_XFontSet: { - INT16 len = data[0]; + CARD16 len = data[0]; char *base_name; XFontSet rep = (XFontSet)NULL; char **missing_list = NULL; @@ -332,11 +332,12 @@ _XimAttributeToValue( return False; if (!ic) return False; - + if (len > data_len) + return False; if (!(base_name = Xmalloc(len + 1))) return False; - (void)strncpy(base_name, (char *)&data[1], (int)len); + (void)strncpy(base_name, (char *)&data[1], (size_t)len); base_name[len] = '\0'; if (mode & XIM_PREEDIT_ATTR) { ++++++ U_004-FixSignedLengthValuesIn_XimGetAttributeID.patch ++++++ From: Todd Carson <[email protected]> The lengths are unsigned according to the specification. Passing negative values can lead to data corruption. Signed-off-by: Matthieu Herrb <[email protected]> Reviewed-by: Matthieu Herrb <[email protected]> --- modules/im/ximcp/imRmAttr.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) Index: libX11-1.6.5/modules/im/ximcp/imRmAttr.c =================================================================== --- libX11-1.6.5.orig/modules/im/ximcp/imRmAttr.c +++ libX11-1.6.5/modules/im/ximcp/imRmAttr.c @@ -1393,13 +1393,13 @@ _XimEncodeSavedICATTRIBUTE( static unsigned int _XimCountNumberOfAttr( - INT16 total, - CARD16 *attr, - int *names_len) + CARD16 total, + CARD16 *attr, + unsigned int *names_len) { unsigned int n; - INT16 len; - INT16 min_len = sizeof(CARD16) /* sizeof attribute ID */ + CARD16 len; + CARD16 min_len = sizeof(CARD16) /* sizeof attribute ID */ + sizeof(CARD16) /* sizeof type of value */ + sizeof(INT16); /* sizeof length of attribute */ @@ -1407,6 +1407,9 @@ _XimCountNumberOfAttr( *names_len = 0; while (total > min_len) { len = attr[2]; + if (len >= (total - min_len)) { + return 0; + } *names_len += (len + 1); len += (min_len + XIM_PAD(len + 2)); total -= len; @@ -1421,17 +1424,15 @@ _XimGetAttributeID( Xim im, CARD16 *buf) { - unsigned int n; + unsigned int n, names_len, values_len; XIMResourceList res; char *names; - int names_len; XPointer tmp; XIMValuesList *values_list; char **values; - int values_len; register int i; - INT16 len; - INT16 min_len = sizeof(CARD16) /* sizeof attribute ID */ + CARD16 len; + CARD16 min_len = sizeof(CARD16) /* sizeof attribute ID */ + sizeof(CARD16) /* sizeof type of value */ + sizeof(INT16); /* sizeof length of attr */ /* ++++++ U_005-ZeroOutBuffersInFunctions.patch ++++++ From: Todd Carson <[email protected]> It looks like uninitialized stack or heap memory can leak out via padding bytes. Signed-off-by: Matthieu Herrb <[email protected]> Reviewed-by: Matthieu Herrb <[email protected]> --- modules/im/ximcp/imDefIc.c | 6 ++++-- modules/im/ximcp/imDefIm.c | 25 +++++++++++++++++-------- 2 files changed, 21 insertions(+), 10 deletions(-) Index: libX11-1.6.5/modules/im/ximcp/imDefIc.c =================================================================== --- libX11-1.6.5.orig/modules/im/ximcp/imDefIc.c +++ libX11-1.6.5/modules/im/ximcp/imDefIc.c @@ -351,7 +351,7 @@ _XimProtoGetICValues( + sizeof(INT16) + XIM_PAD(2 + buf_size); - if (!(buf = Xmalloc(buf_size))) + if (!(buf = Xcalloc(buf_size, 1))) return arg->name; buf_s = (CARD16 *)&buf[XIM_HEADER_SIZE]; @@ -709,6 +709,7 @@ _XimProtoSetICValues( #endif /* XIM_CONNECTABLE */ _XimGetCurrentICValues(ic, &ic_values); + memset(tmp_buf, 0, sizeof(tmp_buf32)); buf = tmp_buf; buf_size = XIM_HEADER_SIZE + sizeof(CARD16) + sizeof(CARD16) + sizeof(INT16) + sizeof(CARD16); @@ -731,7 +732,7 @@ _XimProtoSetICValues( buf_size += ret_len; if (buf == tmp_buf) { - if (!(tmp = Xmalloc(buf_size + data_len))) { + if (!(tmp = Xcalloc(buf_size + data_len, 1))) { return tmp_name; } memcpy(tmp, buf, buf_size); @@ -741,6 +742,7 @@ _XimProtoSetICValues( Xfree(buf); return tmp_name; } + memset(&tmp[buf_size], 0, data_len); buf = tmp; } } Index: libX11-1.6.5/modules/im/ximcp/imDefIm.c =================================================================== --- libX11-1.6.5.orig/modules/im/ximcp/imDefIm.c +++ libX11-1.6.5/modules/im/ximcp/imDefIm.c @@ -62,6 +62,7 @@ PERFORMANCE OF THIS SOFTWARE. #include "XimTrInt.h" #include "Ximint.h" +#include <limits.h> int _XimCheckDataSize( @@ -809,12 +810,16 @@ _XimOpen( int buf_size; int ret_code; char *locale_name; + size_t locale_len; locale_name = im->private.proto.locale_name; - len = strlen(locale_name); - buf_b[0] = (BYTE)len; /* length of locale name */ - (void)strcpy((char *)&buf_b[1], locale_name); /* locale name */ - len += sizeof(BYTE); /* sizeof length */ + locale_len = strlen(locale_name); + if (locale_len > UCHAR_MAX) + return False; + memset(buf32, 0, sizeof(buf32)); + buf_b[0] = (BYTE)locale_len; /* length of locale name */ + memcpy(&buf_b[1], locale_name, locale_len); /* locale name */ + len = (INT16)(locale_len + sizeof(BYTE)); /* sizeof length */ XIM_SET_PAD(buf_b, len); /* pad */ _XimSetHeader((XPointer)buf, XIM_OPEN, 0, &len); @@ -1289,6 +1294,7 @@ _XimProtoSetIMValues( #endif /* XIM_CONNECTABLE */ _XimGetCurrentIMValues(im, &im_values); + memset(tmp_buf, 0, sizeof(tmp_buf32)); buf = tmp_buf; buf_size = XIM_HEADER_SIZE + sizeof(CARD16) + sizeof(INT16); data_len = BUFSIZE - buf_size; @@ -1311,7 +1317,7 @@ _XimProtoSetIMValues( buf_size += ret_len; if (buf == tmp_buf) { - if (!(tmp = Xmalloc(buf_size + data_len))) { + if (!(tmp = Xcalloc(buf_size + data_len, 1))) { return arg->name; } memcpy(tmp, buf, buf_size); @@ -1321,6 +1327,7 @@ _XimProtoSetIMValues( Xfree(buf); return arg->name; } + memset(&tmp[buf_size], 0, data_len); buf = tmp; } } @@ -1462,7 +1469,7 @@ _XimProtoGetIMValues( + sizeof(INT16) + XIM_PAD(buf_size); - if (!(buf = Xmalloc(buf_size))) + if (!(buf = Xcalloc(buf_size, 1))) return arg->name; buf_s = (CARD16 *)&buf[XIM_HEADER_SIZE]; @@ -1724,7 +1731,7 @@ _XimEncodingNegotiation( + sizeof(CARD16) + detail_len; - if (!(buf = Xmalloc(XIM_HEADER_SIZE + len))) + if (!(buf = Xcalloc(XIM_HEADER_SIZE + len, 1))) goto free_detail_ptr; buf_s = (CARD16 *)&buf[XIM_HEADER_SIZE]; @@ -1820,6 +1827,7 @@ _XimSendSavedIMValues( int ret_code; _XimGetCurrentIMValues(im, &im_values); + memset(tmp_buf, 0, sizeof(tmp_buf32)); buf = tmp_buf; buf_size = XIM_HEADER_SIZE + sizeof(CARD16) + sizeof(INT16); data_len = BUFSIZE - buf_size; @@ -1842,7 +1850,7 @@ _XimSendSavedIMValues( buf_size += ret_len; if (buf == tmp_buf) { - if (!(tmp = Xmalloc(buf_size + data_len))) { + if (!(tmp = Xcalloc(buf_size + data_len, 1))) { return False; } memcpy(tmp, buf, buf_size); @@ -1852,6 +1860,7 @@ _XimSendSavedIMValues( Xfree(buf); return False; } + memset(&tmp[buf_size], 0, data_len); buf = tmp; } } ++++++ U_006-Fix-size-calculation-in-_XimAttributeToValue.patch ++++++ >From 93fce3f4e79cbc737d6468a4f68ba3de1b83953b Mon Sep 17 00:00:00 2001 From: Yichao Yu <[email protected]> Date: Sun, 2 Aug 2020 13:43:58 -0400 Subject: [PATCH] Fix size calculation in `_XimAttributeToValue`. The check here guards the read below. For `XimType_XIMStyles`, these are `num` of `CARD32` and for `XimType_XIMHotKeyTriggers` these are `num` of `XIMTRIGGERKEY` ref[1] which is defined as 3 x `CARD32`. (There are data after the `XIMTRIGGERKEY` according to the spec but they are not read by this function and doesn't need to be checked.) The old code here used the native datatype size instead of the wire protocol size causing the check to always fail. Also fix the size calculation for the header (size). It is 2 x CARD16 for both types despite the unused `CARD16` for `XimType_XIMStyles`. [1] https://www.x.org/releases/X11R7.6/doc/libX11/specs/XIM/xim.html#Input_Method_Styles This fixes a regression caused by 388b303c62aa35a245f1704211a023440ad2c488 in 1.6.10. Fix #116 --- modules/im/ximcp/imRmAttr.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/im/ximcp/imRmAttr.c b/modules/im/ximcp/imRmAttr.c index 2491908e7091..919c5564718c 100644 --- a/modules/im/ximcp/imRmAttr.c +++ b/modules/im/ximcp/imRmAttr.c @@ -265,7 +265,7 @@ _XimAttributeToValue( if (num > (USHRT_MAX / sizeof(XIMStyle))) return False; - if ((sizeof(num) + (num * sizeof(XIMStyle))) > data_len) + if ((2 * sizeof(CARD16) + (num * sizeof(CARD32))) > data_len) return False; alloc_len = sizeof(XIMStyles) + sizeof(XIMStyle) * num; if (alloc_len < sizeof(XIMStyles)) @@ -379,7 +379,7 @@ _XimAttributeToValue( if (num > (UINT_MAX / sizeof(XIMHotKeyTrigger))) return False; - if ((sizeof(num) + (num * sizeof(XIMHotKeyTrigger))) > data_len) + if ((2 * sizeof(CARD16) + (num * 3 * sizeof(CARD32))) > data_len) return False; alloc_len = sizeof(XIMHotKeyTriggers) + sizeof(XIMHotKeyTrigger) * num; -- 2.16.4
