vncconfig: add option to force view-only remote client connections

Resolves: RHEL-12144
This commit is contained in:
Jan Grulich 2024-05-28 11:50:49 +02:00
parent d8901da547
commit f76f6574de
8 changed files with 1123 additions and 366 deletions

File diff suppressed because it is too large Load Diff

View File

@ -5,7 +5,7 @@
Name: tigervnc
Version: 1.13.1
Release: 9%{?dist}
Release: 10%{?dist}
Summary: A TigerVNC remote display system
%global _hardened_build 1
@ -28,6 +28,7 @@ Patch2: tigervnc-vncsession-restore-script-systemd-service.patch
# Upstream patches
Patch50: tigervnc-support-username-alias-in-plainusers.patch
Patch51: tigervnc-use-dup-to-get-available-fd-for-inetd.patch
Patch52: tigervnc-add-option-to-force-view-only-remote-connections.patch
# Upstreamable patches
Patch80: tigervnc-dont-get-pointer-position-for-floating-device.patch
@ -38,14 +39,7 @@ Patch100: tigervnc-xserver120.patch
Patch101: 0001-rpath-hack.patch
# XServer patches
# CVE-2024-0229
# https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1251
Patch200: xorg-CVE-2024-0229-followup.patch
Patch201: xorg-CVE-2024-31080.patch
Patch202: xorg-CVE-2024-31081.patch
Patch203: xorg-CVE-2024-31082.patch
Patch204: xorg-CVE-2024-31083.patch
Patch205: xorg-CVE-2024-31083-followup.patch
BuildRequires: make
BuildRequires: gcc-c++
@ -193,22 +187,19 @@ pushd unix/xserver
for all in `find . -type f -perm -001`; do
chmod -x "$all"
done
# Xorg patches
%patch100 -p1 -b .xserver120-rebased
%patch101 -p1 -b .rpath
%patch200 -p1 -b .xorg-CVE-2024-0229-followup
%patch201 -p1 -b .xorg-CVE-2024-31080.patch
%patch202 -p1 -b .xorg-CVE-2024-31081.patch
%patch203 -p1 -b .xorg-CVE-2024-31082.patch
%patch204 -p1 -b .xorg-CVE-2024-31083.patch
%patch205 -p1 -b .xorg-CVE-2024-31083-followup.patch
popd
# Tigervnc patches
%patch1 -p1 -b .use-gnome-as-default-session
%patch2 -p1 -b .vncsession-restore-script-systemd-service
# Upstream patches
%patch50 -p1 -b .support-username-alias-in-plainusers
%patch51 -p1 -b .use-dup-to-get-available-fd-for-inetd
%patch52 -p1 -b .add-option-to-force-view-only-remote-connections
# Upstreamable patches
%patch80 -p1 -b .dont-get-pointer-position-for-floating-device
@ -394,6 +385,10 @@ fi
%ghost %verify(not md5 size mode mtime) %{_sharedstatedir}/selinux/%{selinuxtype}/active/modules/200/%{modulename}
%changelog
* Tue May 28 2024 Jan Grulich <jgrulich@redhat.com> - 1.13.1-10
- vncconfig: add option to force view-only remote client connections
Resolves: RHEL-12144
* Tue Apr 16 2024 Jan Grulich <jgrulich@redhat.com> - 1.13.1-9
- Fix CVE-2024-31080 tigervnc: xorg-x11-server: Heap buffer overread/data leakage in ProcXIGetSelectedEvents
Resolves: RHEL-30756

View File

@ -1,32 +0,0 @@
From 133e0d651c5d12bf01999d6289e84e224ba77adc Mon Sep 17 00:00:00 2001
From: Peter Hutterer <peter.hutterer@who-t.net>
Date: Mon, 22 Jan 2024 14:22:12 +1000
Subject: [PATCH] dix: fix valuator copy/paste error in the DeviceStateNotify
event
Fixes 219c54b8a3337456ce5270ded6a67bcde53553d5
---
dix/enterleave.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/dix/enterleave.c b/dix/enterleave.c
index 7b7ba1098b..c1e6ac600e 100644
--- a/dix/enterleave.c
+++ b/dix/enterleave.c
@@ -619,11 +619,11 @@ FixDeviceValuator(DeviceIntPtr dev, deviceValuator * ev, ValuatorClassPtr v,
ev->first_valuator = first;
switch (ev->num_valuators) {
case 6:
- ev->valuator2 = v->axisVal[first + 5];
+ ev->valuator5 = v->axisVal[first + 5];
case 5:
- ev->valuator2 = v->axisVal[first + 4];
+ ev->valuator4 = v->axisVal[first + 4];
case 4:
- ev->valuator2 = v->axisVal[first + 3];
+ ev->valuator3 = v->axisVal[first + 3];
case 3:
ev->valuator2 = v->axisVal[first + 2];
case 2:
--
GitLab

View File

@ -1,45 +0,0 @@
From 96798fc1967491c80a4d0c8d9e0a80586cb2152b Mon Sep 17 00:00:00 2001
From: Alan Coopersmith <alan.coopersmith@oracle.com>
Date: Fri, 22 Mar 2024 18:51:45 -0700
Subject: [PATCH 1/4] Xi: ProcXIGetSelectedEvents needs to use unswapped length
to send reply
CVE-2024-31080
Reported-by: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=69762
Fixes: 53e821ab4 ("Xi: add request processing for XIGetSelectedEvents.")
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1463>
---
Xi/xiselectev.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/Xi/xiselectev.c b/Xi/xiselectev.c
index edcb8a0d3..ac1494987 100644
--- a/Xi/xiselectev.c
+++ b/Xi/xiselectev.c
@@ -349,6 +349,7 @@ ProcXIGetSelectedEvents(ClientPtr client)
InputClientsPtr others = NULL;
xXIEventMask *evmask = NULL;
DeviceIntPtr dev;
+ uint32_t length;
REQUEST(xXIGetSelectedEventsReq);
REQUEST_SIZE_MATCH(xXIGetSelectedEventsReq);
@@ -418,10 +419,12 @@ ProcXIGetSelectedEvents(ClientPtr client)
}
}
+ /* save the value before SRepXIGetSelectedEvents swaps it */
+ length = reply.length;
WriteReplyToClient(client, sizeof(xXIGetSelectedEventsReply), &reply);
if (reply.num_masks)
- WriteToClient(client, reply.length * 4, buffer);
+ WriteToClient(client, length * 4, buffer);
free(buffer);
return Success;
--
2.44.0

View File

@ -1,43 +0,0 @@
From 3e77295f888c67fc7645db5d0c00926a29ffecee Mon Sep 17 00:00:00 2001
From: Alan Coopersmith <alan.coopersmith@oracle.com>
Date: Fri, 22 Mar 2024 18:56:27 -0700
Subject: [PATCH 2/4] Xi: ProcXIPassiveGrabDevice needs to use unswapped length
to send reply
CVE-2024-31081
Fixes: d220d6907 ("Xi: add GrabButton and GrabKeysym code.")
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1463>
---
Xi/xipassivegrab.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/Xi/xipassivegrab.c b/Xi/xipassivegrab.c
index c9ac2f855..896233bec 100644
--- a/Xi/xipassivegrab.c
+++ b/Xi/xipassivegrab.c
@@ -93,6 +93,7 @@ ProcXIPassiveGrabDevice(ClientPtr client)
GrabParameters param;
void *tmp;
int mask_len;
+ uint32_t length;
REQUEST(xXIPassiveGrabDeviceReq);
REQUEST_FIXED_SIZE(xXIPassiveGrabDeviceReq,
@@ -247,9 +248,11 @@ ProcXIPassiveGrabDevice(ClientPtr client)
}
}
+ /* save the value before SRepXIPassiveGrabDevice swaps it */
+ length = rep.length;
WriteReplyToClient(client, sizeof(rep), &rep);
if (rep.num_modifiers)
- WriteToClient(client, rep.length * 4, modifiers_failed);
+ WriteToClient(client, length * 4, modifiers_failed);
out:
free(modifiers_failed);
--
2.44.0

View File

@ -1,47 +0,0 @@
From 6c684d035c06fd41c727f0ef0744517580864cef Mon Sep 17 00:00:00 2001
From: Alan Coopersmith <alan.coopersmith@oracle.com>
Date: Fri, 22 Mar 2024 19:07:34 -0700
Subject: [PATCH 3/4] Xquartz: ProcAppleDRICreatePixmap needs to use unswapped
length to send reply
CVE-2024-31082
Fixes: 14205ade0 ("XQuartz: appledri: Fix byte swapping in replies")
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1463>
---
hw/xquartz/xpr/appledri.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/hw/xquartz/xpr/appledri.c b/hw/xquartz/xpr/appledri.c
index 77574655b..40422b61a 100644
--- a/hw/xquartz/xpr/appledri.c
+++ b/hw/xquartz/xpr/appledri.c
@@ -272,6 +272,7 @@ ProcAppleDRICreatePixmap(ClientPtr client)
xAppleDRICreatePixmapReply rep;
int width, height, pitch, bpp;
void *ptr;
+ CARD32 stringLength;
REQUEST_SIZE_MATCH(xAppleDRICreatePixmapReq);
@@ -307,6 +308,7 @@ ProcAppleDRICreatePixmap(ClientPtr client)
if (sizeof(rep) != sz_xAppleDRICreatePixmapReply)
ErrorF("error sizeof(rep) is %zu\n", sizeof(rep));
+ stringLength = rep.stringLength; /* save unswapped value */
if (client->swapped) {
swaps(&rep.sequenceNumber);
swapl(&rep.length);
@@ -319,7 +321,7 @@ ProcAppleDRICreatePixmap(ClientPtr client)
}
WriteToClient(client, sizeof(rep), &rep);
- WriteToClient(client, rep.stringLength, path);
+ WriteToClient(client, stringLength, path);
return Success;
}
--
2.44.0

View File

@ -1,72 +0,0 @@
From 337d8d48b618d4fc0168a7b978be4c3447650b04 Mon Sep 17 00:00:00 2001
From: Olivier Fourdan <ofourdan@redhat.com>
Date: Fri, 5 Apr 2024 15:24:49 +0200
Subject: [PATCH] render: Avoid possible double-free in ProcRenderAddGlyphs()
ProcRenderAddGlyphs() adds the glyph to the glyphset using AddGlyph() and
then frees it using FreeGlyph() to decrease the reference count, after
AddGlyph() has increased it.
AddGlyph() however may chose to reuse an existing glyph if it's already
in the glyphSet, and free the glyph that was given, in which case the
caller function, ProcRenderAddGlyphs() will call FreeGlyph() on an
already freed glyph, as reported by ASan:
READ of size 4 thread T0
#0 in FreeGlyph xserver/render/glyph.c:252
#1 in ProcRenderAddGlyphs xserver/render/render.c:1174
#2 in Dispatch xserver/dix/dispatch.c:546
#3 in dix_main xserver/dix/main.c:271
#4 in main xserver/dix/stubmain.c:34
#5 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
#6 in __libc_start_main_impl ../csu/libc-start.c:360
#7 (/usr/bin/Xwayland+0x44fe4)
Address is located 0 bytes inside of 64-byte region
freed by thread T0 here:
#0 in __interceptor_free libsanitizer/asan/asan_malloc_linux.cpp:52
#1 in _dixFreeObjectWithPrivates xserver/dix/privates.c:538
#2 in AddGlyph xserver/render/glyph.c:295
#3 in ProcRenderAddGlyphs xserver/render/render.c:1173
#4 in Dispatch xserver/dix/dispatch.c:546
#5 in dix_main xserver/dix/main.c:271
#6 in main xserver/dix/stubmain.c:34
#7 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
previously allocated by thread T0 here:
#0 in __interceptor_malloc libsanitizer/asan/asan_malloc_linux.cpp:69
#1 in AllocateGlyph xserver/render/glyph.c:355
#2 in ProcRenderAddGlyphs xserver/render/render.c:1085
#3 in Dispatch xserver/dix/dispatch.c:546
#4 in dix_main xserver/dix/main.c:271
#5 in main xserver/dix/stubmain.c:34
#6 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
SUMMARY: AddressSanitizer: heap-use-after-free xserver/render/glyph.c:252 in FreeGlyph
To avoid that, make sure not to free the given glyph in AddGlyph().
v2: Simplify the test using the boolean returned from AddGlyph() (Michel)
v3: Simplify even more by not freeing the glyph in AddGlyph() (Peter)
Fixes: bdca6c3d1 - render: fix refcounting of glyphs during ProcRenderAddGlyphs
Closes: https://gitlab.freedesktop.org/xorg/xserver/-/issues/1659
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1476>
---
render/glyph.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/render/glyph.c b/render/glyph.c
index 13991f8a1..5fa7f3b5b 100644
--- a/render/glyph.c
+++ b/render/glyph.c
@@ -291,8 +291,6 @@ AddGlyph(GlyphSetPtr glyphSet, GlyphPtr glyph, Glyph id)
gr = FindGlyphRef(&globalGlyphs[glyphSet->fdepth], signature,
TRUE, glyph->sha1);
if (gr->glyph && gr->glyph != DeletedGlyph && gr->glyph != glyph) {
- FreeGlyphPicture(glyph);
- dixFreeObjectWithPrivates(glyph, PRIVATE_GLYPH);
glyph = gr->glyph;
}
else if (gr->glyph != glyph) {
--
2.44.0

View File

@ -1,112 +0,0 @@
From bdca6c3d1f5057eeb31609b1280fc93237b00c77 Mon Sep 17 00:00:00 2001
From: Peter Hutterer <peter.hutterer@who-t.net>
Date: Tue, 30 Jan 2024 13:13:35 +1000
Subject: [PATCH 4/4] render: fix refcounting of glyphs during
ProcRenderAddGlyphs
Previously, AllocateGlyph would return a new glyph with refcount=0 and a
re-used glyph would end up not changing the refcount at all. The
resulting glyph_new array would thus have multiple entries pointing to
the same non-refcounted glyphs.
AddGlyph may free a glyph, resulting in a UAF when the same glyph
pointer is then later used.
Fix this by returning a refcount of 1 for a new glyph and always
incrementing the refcount for a re-used glyph, followed by dropping that
refcount back down again when we're done with it.
CVE-2024-31083, ZDI-CAN-22880
This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1463>
---
render/glyph.c | 5 +++--
render/glyphstr_priv.h | 1 +
render/render.c | 15 +++++++++++----
3 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/render/glyph.c b/render/glyph.c
index 850ea8440..13991f8a1 100644
--- a/render/glyph.c
+++ b/render/glyph.c
@@ -245,10 +245,11 @@ FreeGlyphPicture(GlyphPtr glyph)
}
}
-static void
+void
FreeGlyph(GlyphPtr glyph, int format)
{
CheckDuplicates(&globalGlyphs[format], "FreeGlyph");
+ BUG_RETURN(glyph->refcnt == 0);
if (--glyph->refcnt == 0) {
GlyphRefPtr gr;
int i;
@@ -354,7 +355,7 @@ AllocateGlyph(xGlyphInfo * gi, int fdepth)
glyph = (GlyphPtr) malloc(size);
if (!glyph)
return 0;
- glyph->refcnt = 0;
+ glyph->refcnt = 1;
glyph->size = size + sizeof(xGlyphInfo);
glyph->info = *gi;
dixInitPrivates(glyph, (char *) glyph + head_size, PRIVATE_GLYPH);
diff --git a/render/glyphstr.h b/render/glyphstr.h
index 2f51bd244..3b1d806d1 100644
--- a/render/glyphstr.h
+++ b/render/glyphstr.h
@@ -108,6 +108,7 @@ extern Bool
extern GlyphPtr FindGlyph(GlyphSetPtr glyphSet, Glyph id);
extern GlyphPtr AllocateGlyph(xGlyphInfo * gi, int format);
+extern void FreeGlyph(GlyphPtr glyph, int format);
extern Bool
ResizeGlyphSet(GlyphSetPtr glyphSet, CARD32 change);
diff --git a/render/render.c b/render/render.c
index 29c5055c6..fe5e37dd9 100644
--- a/render/render.c
+++ b/render/render.c
@@ -1076,6 +1076,7 @@ ProcRenderAddGlyphs(ClientPtr client)
if (glyph_new->glyph && glyph_new->glyph != DeletedGlyph) {
glyph_new->found = TRUE;
+ ++glyph_new->glyph->refcnt;
}
else {
GlyphPtr glyph;
@@ -1168,8 +1169,10 @@ ProcRenderAddGlyphs(ClientPtr client)
err = BadAlloc;
goto bail;
}
- for (i = 0; i < nglyphs; i++)
+ for (i = 0; i < nglyphs; i++) {
AddGlyph(glyphSet, glyphs[i].glyph, glyphs[i].id);
+ FreeGlyph(glyphs[i].glyph, glyphSet->fdepth);
+ }
if (glyphsBase != glyphsLocal)
free(glyphsBase);
@@ -1179,9 +1182,13 @@ ProcRenderAddGlyphs(ClientPtr client)
FreePicture((void *) pSrc, 0);
if (pSrcPix)
FreeScratchPixmapHeader(pSrcPix);
- for (i = 0; i < nglyphs; i++)
- if (glyphs[i].glyph && !glyphs[i].found)
- free(glyphs[i].glyph);
+ for (i = 0; i < nglyphs; i++) {
+ if (glyphs[i].glyph) {
+ --glyphs[i].glyph->refcnt;
+ if (!glyphs[i].found)
+ free(glyphs[i].glyph);
+ }
+ }
if (glyphsBase != glyphsLocal)
free(glyphsBase);
return err;
--
2.44.0