diff options
author | Peter Hutterer <peter.hutterer@who-t.net> | 2024-01-30 13:13:35 +1000 |
---|---|---|
committer | Povilas Kanapickas <povilas@radix.lt> | 2024-04-03 19:37:08 +0300 |
commit | 1173156404be826f50f453ca11bda28ccb5a5268 (patch) | |
tree | 919c0dbba46be01e50011452ed3f6c8860ff3a2c | |
parent | 0e34d8ebc98a0ba6f9f0a2f8f5045761bccc45d3 (diff) |
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>
(cherry picked from commit bdca6c3d1f5057eeb31609b1280fc93237b00c77)
-rw-r--r-- | render/glyph.c | 5 | ||||
-rw-r--r-- | render/glyphstr.h | 2 | ||||
-rw-r--r-- | render/render.c | 15 |
3 files changed, 16 insertions, 6 deletions
diff --git a/render/glyph.c b/render/glyph.c index f3ed9cf4c..d5fc5f3c9 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..e8034556d 100644 --- a/render/glyphstr.h +++ b/render/glyphstr.h @@ -109,6 +109,8 @@ 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 456f156d4..5bc2a204b 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; |