- 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)

OBS-URL: https://build.opensuse.org/package/show/X11:XOrg/libX11?expand=0&rev=56
This commit is contained in:
Stefan Dirsch 2020-07-31 20:29:15 +00:00 committed by Git OBS Bridge
parent f090206a3f
commit f6fe37bced
7 changed files with 374 additions and 2 deletions

View File

@ -0,0 +1,23 @@
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 <matthieu@herrb.eu>
Reviewed-by: Todd Carson <toc@daybefore.net>
---
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)
{

View File

@ -0,0 +1,75 @@
From: Todd Carson <tc@daybefore.net>
Signed-off-by: Matthieu Herrb <matthieu@herrb.eu>
Reviewed-by: Matthieu Herrb <matthieu@herrb.eu>
---
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;

View File

@ -0,0 +1,36 @@
From: Todd Carson <tc@daybefore.net>
Signed-off-by: Matthieu Herrb <matthieu@herrb.eu>
Reviewed-by: Matthieu Herrb <matthieu@herrb.eu>
---
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) {

View File

@ -0,0 +1,65 @@
From: Todd Carson <tc@daybefore.net>
The lengths are unsigned according to the specification. Passing
negative values can lead to data corruption.
Signed-off-by: Matthieu Herrb <matthieu@herrb.eu>
Reviewed-by: Matthieu Herrb <matthieu@herrb.eu>
---
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 */
/*

View File

@ -0,0 +1,151 @@
From: Todd Carson <tc@daybefore.net>
It looks like uninitialized stack or heap memory can leak
out via padding bytes.
Signed-off-by: Matthieu Herrb <matthieu@herrb.eu>
Reviewed-by: Matthieu Herrb <matthieu@herrb.eu>
---
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;
}
}

View File

@ -1,3 +1,13 @@
-------------------------------------------------------------------
Fri Jul 31 20:23:05 UTC 2020 - Stefan Dirsch <sndirsch@suse.com>
- 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)
-------------------------------------------------------------------
Sun Oct 20 18:27:32 UTC 2019 - Stefan Brüns <stefan.bruens@rwth-aachen.de>

View File

@ -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,11 @@ Patch0: p_khmer-compose.diff
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
BuildRequires: fdupes
BuildRequires: libtool
BuildRequires: pkgconfig
@ -133,7 +138,14 @@ in libX11-6 and libX11-xcb1.
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
%build
%configure \