frederik
/
xserver
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

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>
This commit is contained in:
Peter Hutterer 2024-01-30 13:13:35 +10:00 committed by Alan Coopersmith
parent 6c684d035c
commit bdca6c3d1f
3 changed files with 15 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
render/glyph.c
View File

@ -245,10 +245,11 @@ FreeGlyphPicture(GlyphPtr glyph)
} }
} }
static void void
FreeGlyph(GlyphPtr glyph, int format) FreeGlyph(GlyphPtr glyph, int format)
{ {
CheckDuplicates(&globalGlyphs[format], "FreeGlyph"); CheckDuplicates(&globalGlyphs[format], "FreeGlyph");
BUG_RETURN(glyph->refcnt == 0);
if (--glyph->refcnt == 0) { if (--glyph->refcnt == 0) {
GlyphRefPtr gr; GlyphRefPtr gr;
int i; int i;
@ -354,7 +355,7 @@ AllocateGlyph(xGlyphInfo * gi, int fdepth)
glyph = (GlyphPtr) malloc(size); glyph = (GlyphPtr) malloc(size);
if (!glyph) if (!glyph)
return 0; return 0;
glyph->refcnt = 0; glyph->refcnt = 1;
glyph->size = size + sizeof(xGlyphInfo); glyph->size = size + sizeof(xGlyphInfo);
glyph->info = *gi; glyph->info = *gi;
dixInitPrivates(glyph, (char *) glyph + head_size, PRIVATE_GLYPH); dixInitPrivates(glyph, (char *) glyph + head_size, PRIVATE_GLYPH);

1
render/glyphstr_priv.h
View File

@ -56,6 +56,7 @@ void AddGlyph(GlyphSetPtr glyphSet, GlyphPtr glyph, Glyph id);
Bool DeleteGlyph(GlyphSetPtr glyphSet, Glyph id); Bool DeleteGlyph(GlyphSetPtr glyphSet, Glyph id);
GlyphPtr FindGlyph(GlyphSetPtr glyphSet, Glyph id); GlyphPtr FindGlyph(GlyphSetPtr glyphSet, Glyph id);
GlyphPtr AllocateGlyph(xGlyphInfo * gi, int format); GlyphPtr AllocateGlyph(xGlyphInfo * gi, int format);
void FreeGlyph(GlyphPtr glyph, int format);
Bool ResizeGlyphSet(GlyphSetPtr glyphSet, CARD32 change); Bool ResizeGlyphSet(GlyphSetPtr glyphSet, CARD32 change);
GlyphSetPtr AllocateGlyphSet(int fdepth, PictFormatPtr format); GlyphSetPtr AllocateGlyphSet(int fdepth, PictFormatPtr format);
int FreeGlyphSet(void *value, XID gid); int FreeGlyphSet(void *value, XID gid);

15
render/render.c
View File

@ -1076,6 +1076,7 @@ ProcRenderAddGlyphs(ClientPtr client)
if (glyph_new->glyph && glyph_new->glyph != DeletedGlyph) { if (glyph_new->glyph && glyph_new->glyph != DeletedGlyph) {
glyph_new->found = TRUE; glyph_new->found = TRUE;
++glyph_new->glyph->refcnt;
} }
else { else {
GlyphPtr glyph; GlyphPtr glyph;
@ -1168,8 +1169,10 @@ ProcRenderAddGlyphs(ClientPtr client)
err = BadAlloc; err = BadAlloc;
goto bail; goto bail;
} }
for (i = 0; i < nglyphs; i++) for (i = 0; i < nglyphs; i++) {
AddGlyph(glyphSet, glyphs[i].glyph, glyphs[i].id); AddGlyph(glyphSet, glyphs[i].glyph, glyphs[i].id);
FreeGlyph(glyphs[i].glyph, glyphSet->fdepth);
}
if (glyphsBase != glyphsLocal) if (glyphsBase != glyphsLocal)
free(glyphsBase); free(glyphsBase);
@ -1179,9 +1182,13 @@ ProcRenderAddGlyphs(ClientPtr client)
FreePicture((void *) pSrc, 0); FreePicture((void *) pSrc, 0);
if (pSrcPix) if (pSrcPix)
FreeScratchPixmapHeader(pSrcPix); FreeScratchPixmapHeader(pSrcPix);
for (i = 0; i < nglyphs; i++) for (i = 0; i < nglyphs; i++) {
if (glyphs[i].glyph && !glyphs[i].found) if (glyphs[i].glyph) {
free(glyphs[i].glyph); --glyphs[i].glyph->refcnt;
if (!glyphs[i].found)
free(glyphs[i].glyph);
}
}
if (glyphsBase != glyphsLocal) if (glyphsBase != glyphsLocal)
free(glyphsBase); free(glyphsBase);
return err; return err;