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