ea7d05a241
Resolves: RHEL-30755 Fix CVE-2024-31083 tigervnc: xorg-x11-server: User-after-free in ProcRenderAddGlyphs Resolves: RHEL-30767 Fix CVE-2024-31081 tigervnc: xorg-x11-server: Heap buffer overread/data leakage in ProcXIPassiveGrabDevice Resolves: RHEL-30761
113 lines
3.6 KiB
Diff
113 lines
3.6 KiB
Diff
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
|
|
|