[PATCH v2] render: Fix crash in RenderAddGlyphs (#23645)

Peter Hutterer peter.hutterer at who-t.net
Tue Sep 29 22:15:21 PDT 2009


This patch fixes two bugs:
size is calculated as glyph height * padded_width. If the client submits
garbage, this may get above INT_MAX, resulting in a negative size if size is
unsigned. The sanity checks don't trigger for negative sizes and the server
goes and writes into random memory locations.

If the client submits glyphs with a width or height 0, the destination
pixmap is NULL, causing a null-pointer dereference. Since there's nothing to
composite if the width/height is 0, we might as well skip the whole thing
anyway.

Tested with Xvfb, Xephyr and Xorg.

X.Org Bug 23645 <http://bugs.freedesktop.org/show_bug.cgi?id=23645>

Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
---

Next version, Hunks 1&2 are new. Hunk 2 ensures that the glyph picture is
NULL (it doesn't get set due to skipping the loop), Hunk 1 avoids the
resulting crash when trying to free a NULL pixmap..

miGlyphs seems to deal with 0-width glyphs just fine

 render/glyph.c  |    4 +++-
 render/render.c |    6 +++++-
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/render/glyph.c b/render/glyph.c
index 6327c9f..55e1827 100644
--- a/render/glyph.c
+++ b/render/glyph.c
@@ -282,7 +282,8 @@ FreeGlyphPicture(GlyphPtr glyph)
     {
         ScreenPtr pScreen = screenInfo.screens[i];
 
-        FreePicture ((pointer) GlyphPicture (glyph)[i], 0);
+        if (GlyphPicture(glyph)[i])
+            FreePicture ((pointer) GlyphPicture (glyph)[i], 0);
 
         ps = GetPictureScreenIfSet (pScreen);
         if (ps)
@@ -414,6 +415,7 @@ AllocateGlyph (xGlyphInfo *gi, int fdepth)
 
     for (i = 0; i < screenInfo.numScreens; i++)
     {
+	GlyphPicture(glyph)[i] = NULL;
 	ps = GetPictureScreenIfSet (screenInfo.screens[i]);
 
 	if (ps)
diff --git a/render/render.c b/render/render.c
index a306766..44e9910 100644
--- a/render/render.c
+++ b/render/render.c
@@ -1043,7 +1043,7 @@ ProcRenderAddGlyphs (ClientPtr client)
     CARD32	    *gids;
     xGlyphInfo	    *gi;
     CARD8	    *bits;
-    int		    size;
+    unsigned int    size;
     int		    err;
     int		    i, screen;
     PicturePtr	    pSrc = NULL, pDst = NULL;
@@ -1131,6 +1131,10 @@ ProcRenderAddGlyphs (ClientPtr client)
 		ScreenPtr   pScreen;
 		int	    error;
 
+		/* Skip work if it's invisibly small anyway */
+		if (!width || !height)
+		    break;
+
 		pScreen = screenInfo.screens[screen];
 		pSrcPix = GetScratchPixmapHeader (pScreen,
 						  width, height,
-- 
1.6.3.rc1.2.g0164.dirty


More information about the xorg-devel mailing list