Fix RHEL SAST Automation errors

Upstream merge request:
https://gitlab.freedesktop.org/xorg/lib/libx11/-/merge_requests/250

Resolves: https://issues.redhat.com/browse/RHEL-34918
This commit is contained in:
José Expósito 2024-06-20 10:37:10 +02:00
parent cfbe5d4947
commit 26c6b983e8
7 changed files with 343 additions and 1 deletions

View File

@ -0,0 +1,49 @@
From 4f5541193dd5a004ed5ea44c12fc25e227113c9b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jos=C3=A9=20Exp=C3=B3sito?= <jexposit@redhat.com>
Date: Tue, 30 Apr 2024 16:37:21 +0200
Subject: [PATCH 1/6] Fix use of uninitialized variable in _XimTriggerNotify
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
`_XimRead()` is being called with `reply` as target buffer instead of
using `preply`, accessing uninitialized memory a few lines later.
This error has been found by a static analysis tool. This is the report:
Error: UNINIT (CWE-457):
libX11-1.8.7/modules/im/ximcp/imDefLkup.c:561: alloc_fn:
Calling "malloc" which returns uninitialized memory.
libX11-1.8.7/modules/im/ximcp/imDefLkup.c:561: assign:
Assigning: "preply" = "malloc((size_t)((len == 0) ? 1 : len))",
which points to uninitialized data.
libX11-1.8.7/modules/im/ximcp/imDefLkup.c:573: uninit_use:
Using uninitialized value "*((CARD8 *)preply)".
# 571| }
# 572| buf_s = (CARD16 *)((char *)preply + XIM_HEADER_SIZE);
# 573|-> if (*((CARD8 *)preply) == XIM_ERROR) {
# 574| _XimProcError(im, 0, (XPointer)&buf_s[3]);
# 575| if(reply != preply)
Signed-off-by: José Expósito <jexposit@redhat.com>
Part-of: <https://gitlab.freedesktop.org/xorg/lib/libx11/-/merge_requests/250>
---
modules/im/ximcp/imDefLkup.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/modules/im/ximcp/imDefLkup.c b/modules/im/ximcp/imDefLkup.c
index 2e53ab23..8ccaee26 100644
--- a/modules/im/ximcp/imDefLkup.c
+++ b/modules/im/ximcp/imDefLkup.c
@@ -635,7 +635,7 @@ _XimTriggerNotify(
} else {
buf_size = len;
preply = Xmalloc(len);
- ret_code = _XimRead(im, &len, (XPointer)reply, buf_size,
+ ret_code = _XimRead(im, &len, preply, buf_size,
_XimTriggerNotifyCheck, (XPointer)ic);
if(ret_code != XIM_TRUE) {
Xfree(preply);
--
2.45.2

View File

@ -0,0 +1,49 @@
From eaad761e24722b1743d3edee3383294bfb4947d6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jos=C3=A9=20Exp=C3=B3sito?= <jexposit@redhat.com>
Date: Tue, 30 Apr 2024 16:41:40 +0200
Subject: [PATCH 2/6] Fix use of uninitialized variable in _XimExtension
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
`_XimRead()` is being called with `reply` as target buffer instead of
using `preply`, accessing uninitialized memory a few lines later.
This error has been found by a static analysis tool. This is the report:
Error: UNINIT (CWE-457):
libX11-1.8.7/modules/im/ximcp/imExten.c:468: alloc_fn:
Calling "malloc" which returns uninitialized memory.
libX11-1.8.7/modules/im/ximcp/imExten.c:468: assign:
Assigning: "preply" = "malloc((size_t)((buf_size == 0) ? 1 : buf_size))",
which points to uninitialized data.
libX11-1.8.7/modules/im/ximcp/imExten.c:479: uninit_use:
Using uninitialized value "*((CARD8 *)preply)".
# 477| return False;
# 478| buf_s = (CARD16 *)((char *)preply + XIM_HEADER_SIZE);
# 479|-> if (*((CARD8 *)preply) == XIM_ERROR) {
# 480| _XimProcError(im, 0, (XPointer)&buf_s[3]);
# 481| if(reply != preply)
Signed-off-by: José Expósito <jexposit@redhat.com>
Part-of: <https://gitlab.freedesktop.org/xorg/lib/libx11/-/merge_requests/250>
---
modules/im/ximcp/imExten.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/modules/im/ximcp/imExten.c b/modules/im/ximcp/imExten.c
index c2e48a89..a25f00d0 100644
--- a/modules/im/ximcp/imExten.c
+++ b/modules/im/ximcp/imExten.c
@@ -466,7 +466,7 @@ _XimExtension(
} else {
buf_size = len;
preply = Xmalloc(buf_size);
- ret_code = _XimRead(im, &len, reply, buf_size,
+ ret_code = _XimRead(im, &len, preply, buf_size,
_XimQueryExtensionCheck, 0);
if(ret_code != XIM_TRUE) {
Xfree(preply);
--
2.45.2

View File

@ -0,0 +1,47 @@
From 836a8f2cf5e930c8a56b512273fdf9890282ba04 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jos=C3=A9=20Exp=C3=B3sito?= <jexposit@redhat.com>
Date: Tue, 30 Apr 2024 16:49:26 +0200
Subject: [PATCH 3/6] Fix use of uninitialized variable in
_XimEncodeICATTRIBUTE
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
In the `res->resource_size == XimType_NEST` code path, if
`res->xrm_name != pre_quark` and `res->xrm_name != sts_quark`, `len` can
be used uninitialized.
This error has been found by a static analysis tool. This is the report:
Error: UNINIT (CWE-457):
libX11-1.8.7/modules/im/ximcp/imRmAttr.c:1106: var_decl:
Declaring variable "len" without initializer.
libX11-1.8.7/modules/im/ximcp/imRmAttr.c:1179: uninit_use:
Using uninitialized value "len".
# 1177| }
# 1178|
# 1179|-> if (len == 0) {
# 1180| continue;
# 1181| } else if (len < 0) {
Signed-off-by: José Expósito <jexposit@redhat.com>
Part-of: <https://gitlab.freedesktop.org/xorg/lib/libx11/-/merge_requests/250>
---
modules/im/ximcp/imRmAttr.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/modules/im/ximcp/imRmAttr.c b/modules/im/ximcp/imRmAttr.c
index 709e64ab..c56bd62e 100644
--- a/modules/im/ximcp/imRmAttr.c
+++ b/modules/im/ximcp/imRmAttr.c
@@ -1115,6 +1115,7 @@ _XimEncodeICATTRIBUTE(
*ret_len = 0;
for (p = arg; p && p->name; p++) {
+ len = 0;
buf_s = (CARD16 *)buf;
if (!(res = _XimGetResourceListRec(res_list, res_num, p->name))) {
if (_XimSetInnerICAttributes(ic, top, p, mode))
--
2.45.2

View File

@ -0,0 +1,62 @@
From af1312d2873d2ce49b18708a5029895aed477392 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jos=C3=A9=20Exp=C3=B3sito?= <jexposit@redhat.com>
Date: Tue, 30 Apr 2024 17:37:39 +0200
Subject: [PATCH 4/6] XKBMAlloc: Check that needed is >= 0 in
XkbResizeKeyActions
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Passing a negative value in `needed` to the `XkbResizeKeyActions()`
function can create a `newActs` array of an unespected size.
Check the value and return if it is invalid.
This error has been found by a static analysis tool. This is the report:
Error: OVERRUN (CWE-119):
libX11-1.8.7/src/xkb/XKBMAlloc.c:811: cond_const:
Checking "xkb->server->size_acts == 0" implies that
"xkb->server->size_acts" is 0 on the true branch.
libX11-1.8.7/src/xkb/XKBMAlloc.c:811: buffer_alloc:
"calloc" allocates 8 bytes dictated by parameters
"(size_t)((xkb->server->size_acts == 0) ? 1 : xkb->server->size_acts)"
and "8UL".
libX11-1.8.7/src/xkb/XKBMAlloc.c:811: var_assign:
Assigning: "newActs" = "calloc((size_t)((xkb->server->size_acts == 0) ? 1 : xkb->server->size_acts), 8UL)".
libX11-1.8.7/src/xkb/XKBMAlloc.c:815: assignment:
Assigning: "nActs" = "1".
libX11-1.8.7/src/xkb/XKBMAlloc.c:829: cond_at_least:
Checking "nCopy > 0" implies that "nCopy" is at least 1 on the
true branch.
libX11-1.8.7/src/xkb/XKBMAlloc.c:830: overrun-buffer-arg:
Overrunning buffer pointed to by "&newActs[nActs]" of 8 bytes by
passing it to a function which accesses it at byte offset 15
using argument "nCopy * 8UL" (which evaluates to 8).
# 828|
# 829| if (nCopy > 0)
# 830|-> memcpy(&newActs[nActs], XkbKeyActionsPtr(xkb, i),
# 831| nCopy * sizeof(XkbAction));
# 832| if (nCopy < nKeyActs)
Signed-off-by: José Expósito <jexposit@redhat.com>
Part-of: <https://gitlab.freedesktop.org/xorg/lib/libx11/-/merge_requests/250>
---
src/xkb/XKBMAlloc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/xkb/XKBMAlloc.c b/src/xkb/XKBMAlloc.c
index 8b3be303..0563a688 100644
--- a/src/xkb/XKBMAlloc.c
+++ b/src/xkb/XKBMAlloc.c
@@ -795,7 +795,7 @@ XkbResizeKeyActions(XkbDescPtr xkb, int key, int needed)
register int i, nActs;
XkbAction *newActs;
- if (needed == 0) {
+ if (needed <= 0) {
xkb->server->key_acts[key] = 0;
return NULL;
}
--
2.45.2

View File

@ -0,0 +1,64 @@
From f67a87dad40141f50f4da35b28a92a974bfdf7e1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jos=C3=A9=20Exp=C3=B3sito?= <jexposit@redhat.com>
Date: Tue, 30 Apr 2024 18:04:35 +0200
Subject: [PATCH 5/6] Fix memory leak in _XimProtoSetIMValues
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
This error has been found by a static analysis tool. This is the report:
Error: RESOURCE_LEAK (CWE-772):
libX11-1.8.7/modules/im/ximcp/imDefIm.c:1316: alloc_fn:
Storage is returned from allocation function "calloc".
libX11-1.8.7/modules/im/ximcp/imDefIm.c:1316: var_assign:
Assigning: "tmp" = storage returned from
"calloc((size_t)((buf_size + data_len == 0) ? 1 : (buf_size + data_len)), 1UL)".
libX11-1.8.7/modules/im/ximcp/imDefIm.c:1319: noescape:
Resource "tmp" is not freed or pointed-to in "memcpy".
libX11-1.8.7/modules/im/ximcp/imDefIm.c:1320: var_assign:
Assigning: "buf" = "tmp".
libX11-1.8.7/modules/im/ximcp/imDefIm.c:1302: var_assign:
Assigning: "data" = "buf".
libX11-1.8.7/modules/im/ximcp/imDefIm.c:1303: noescape:
Resource "data" is not freed or pointed-to in
"_XimEncodeIMATTRIBUTE".
libX11-1.8.7/modules/im/ximcp/imDefIm.c:1333: leaked_storage:
Variable "data" going out of scope leaks the storage it points to.
libX11-1.8.7/modules/im/ximcp/imDefIm.c:1333: leaked_storage:
Variable "buf" going out of scope leaks the storage it points to.
libX11-1.8.7/modules/im/ximcp/imDefIm.c:1333: leaked_storage:
Variable "tmp" going out of scope leaks the storage it points to.
# 1331|
# 1332| if (!total)
# 1333|-> return (char *)NULL;
# 1334|
# 1335| buf_s = (CARD16 *)&buf[XIM_HEADER_SIZE];
Signed-off-by: José Expósito <jexposit@redhat.com>
Part-of: <https://gitlab.freedesktop.org/xorg/lib/libx11/-/merge_requests/250>
---
modules/im/ximcp/imDefIm.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/modules/im/ximcp/imDefIm.c b/modules/im/ximcp/imDefIm.c
index a12d2970..e3075398 100644
--- a/modules/im/ximcp/imDefIm.c
+++ b/modules/im/ximcp/imDefIm.c
@@ -1327,8 +1327,11 @@ _XimProtoSetIMValues(
}
_XimSetCurrentIMValues(im, &im_values);
- if (!total)
- return (char *)NULL;
+ if (!total) {
+ if (buf != tmp_buf)
+ Xfree(buf);
+ return (char *)NULL;
+ }
buf_s = (CARD16 *)&buf[XIM_HEADER_SIZE];
buf_s[0] = im->private.proto.imid;
--
2.45.2

View File

@ -0,0 +1,57 @@
From 97fb5bda3d0777380cd4b964f48771a82ef3f2a7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jos=C3=A9=20Exp=C3=B3sito?= <jexposit@redhat.com>
Date: Tue, 30 Apr 2024 18:21:08 +0200
Subject: [PATCH 6/6] Fix buffer overrun in parse_omit_name
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
When `num_fields == 12`, if the last character of the pattern is '-',
the `buf` array is overrun.
This error has been found by a static analysis tool. This is the report:
Error: OVERRUN (CWE-119):
libX11-1.8.7/modules/om/generic/omGeneric.c:691: cond_at_most:
Checking "length > 255" implies that "length" may be up to 255 on
the false branch.
libX11-1.8.7/modules/om/generic/omGeneric.c:695: alias:
Assigning: "last" = "buf + length - 1". "last" may now point to as
high as byte 254 of "buf" (which consists of 256 bytes).
libX11-1.8.7/modules/om/generic/omGeneric.c:718: ptr_incr:
Incrementing "last". "last" may now point to as high as byte 255
of "buf" (which consists of 256 bytes).
libX11-1.8.7/modules/om/generic/omGeneric.c:720: ptr_incr:
Incrementing "last". "last" may now point to as high as byte 256
of "buf" (which consists of 256 bytes).
libX11-1.8.7/modules/om/generic/omGeneric.c:720: overrun-local:
Overrunning array of 256 bytes at byte offset 256 by
dereferencing pointer "++last".
# 718| *++last = '*';
# 719|
# 720|-> *++last = '-';
# 721| break;
# 722| case 13:
Signed-off-by: José Expósito <jexposit@redhat.com>
Part-of: <https://gitlab.freedesktop.org/xorg/lib/libx11/-/merge_requests/250>
---
modules/om/generic/omGeneric.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/modules/om/generic/omGeneric.c b/modules/om/generic/omGeneric.c
index 406cec93..370072f3 100644
--- a/modules/om/generic/omGeneric.c
+++ b/modules/om/generic/omGeneric.c
@@ -688,7 +688,7 @@ parse_omit_name(
length = strlen (pattern);
- if (length > XLFD_MAX_LEN)
+ if (length > XLFD_MAX_LEN - 1)
return -1;
strcpy(buf, pattern);
--
2.45.2

View File

@ -5,7 +5,7 @@
Summary: Core X11 protocol client library Summary: Core X11 protocol client library
Name: libX11 Name: libX11
Version: 1.8.7 Version: 1.8.7
Release: 3%{?gitdate:.%{gitdate}git%{gitversion}}%{?dist} Release: 4%{?gitdate:.%{gitdate}git%{gitversion}}%{?dist}
License: MIT AND X11 License: MIT AND X11
URL: http://www.x.org URL: http://www.x.org
@ -25,6 +25,14 @@ Patch03: 0001-Revert-Fix-XTS-regression-in-XCopyColormapAndFree.patch
Patch04: 0002-Revert-Protect-colormap-add-removal-with-display-loc.patch Patch04: 0002-Revert-Protect-colormap-add-removal-with-display-loc.patch
Patch05: 0003-Make-colormap-private-interfaces-thread-safe.patch Patch05: 0003-Make-colormap-private-interfaces-thread-safe.patch
# https://issues.redhat.com/browse/RHEL-34918
Patch06: 0001-Fix-use-of-uninitialized-variable-in-_XimTriggerNoti.patch
Patch07: 0002-Fix-use-of-uninitialized-variable-in-_XimExtension.patch
Patch08: 0003-Fix-use-of-uninitialized-variable-in-_XimEncodeICATT.patch
Patch09: 0004-XKBMAlloc-Check-that-needed-is-0-in-XkbResizeKeyActi.patch
Patch10: 0005-Fix-memory-leak-in-_XimProtoSetIMValues.patch
Patch11: 0006-Fix-buffer-overrun-in-parse_omit_name.patch
BuildRequires: libtool BuildRequires: libtool
BuildRequires: make BuildRequires: make
BuildRequires: xorg-x11-util-macros >= 1.11 BuildRequires: xorg-x11-util-macros >= 1.11
@ -128,6 +136,12 @@ make %{?_smp_mflags} check
%{_mandir}/man5/*.5* %{_mandir}/man5/*.5*
%changelog %changelog
* Thu Jun 20 2024 José Expósito <jexposit@redhat.com> - 1.8.7-4
- Fix XTS test XCopyColormapAndFree/5 hangs
Resolves: https://issues.redhat.com/browse/RHEL-40132
- Fix RHEL SAST Automation errors
Resolves: https://issues.redhat.com/browse/RHEL-34918
* Thu Jan 25 2024 Fedora Release Engineering <releng@fedoraproject.org> - 1.8.7-3 * Thu Jan 25 2024 Fedora Release Engineering <releng@fedoraproject.org> - 1.8.7-3
- Rebuilt for https://fedoraproject.org/wiki/Fedora_40_Mass_Rebuild - Rebuilt for https://fedoraproject.org/wiki/Fedora_40_Mass_Rebuild