pool/libX11
SHA256
2
0
Fork 1
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Accepting request 824353 from X11:XOrg

Browse Source 
- U_006-Fix-size-calculation-in-_XimAttributeToValue.patch:
  * Regression fix in previous XIM client head overflow fixes
    (CVE-2020-14344, bsc#1174628) (forwarded request 824349 from tiwai)

OBS-URL: https://build.opensuse.org/request/show/824353
OBS-URL: https://build.opensuse.org/package/show/openSUSE:Factory/libX11?expand=0&rev=27
This commit is contained in:
Dominique Leuenberger 2020-08-05 18:26:35 +00:00 committed by Git OBS Bridge
commit e9ffee0ba8
8 changed files with 434 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
U_001-ChangeTheData_lenParameterOf_XimAttributeToValueToCARD16.patch Normal file
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)
{

75
U_002-FixIntegerOverflowsIn_XimAttributeToValue.patch Normal file
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;

36
U_003-FixMoreUncheckedLengths.patch Normal file
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) {

65
U_004-FixSignedLengthValuesIn_XimGetAttributeID.patch Normal file
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 */
/*

151
U_005-ZeroOutBuffersInFunctions.patch Normal file
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;
}
}

51
U_006-Fix-size-calculation-in-_XimAttributeToValue.patch Normal file
View File

@ -0,0 +1,51 @@
From 93fce3f4e79cbc737d6468a4f68ba3de1b83953b Mon Sep 17 00:00:00 2001
From: Yichao Yu <yyc1992@gmail.com>
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

17
libX11.changes
View File

@ -1,3 +1,20 @@
-------------------------------------------------------------------
Tue Aug 4 16:33:45 CEST 2020 - tiwai@suse.de
- 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 <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>

18
libX11.spec
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,12 @@ 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
Patch26: U_006-Fix-size-calculation-in-_XimAttributeToValue.patch
BuildRequires: fdupes
BuildRequires: libtool
BuildRequires: pkgconfig
@ -133,7 +139,15 @@ 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
%patch26 -p1
%build
%configure \