reassign 284448 xfree86
retitle 284448 xfree86: font library has very poor bounds-checking and can SEGV 
xfs and the X server
tag 284448 = upstream fixed-upstream patch
thanks

On Fri, Dec 17, 2004 at 12:22:25PM +0100, Thomas Winischhofer wrote:
> This looks like an Xlibs bug.

Yeah, it's one of those annoying static libraries that is linked both into
xfs and the X server.

> From the fact that "pd" is set to a legal value in the debugging 
> output, while "buf" (after adding "pi->data_len") is "out of bounds" I 
> would very much assume that "pi->data_len" contains garbage.
> 
> As regards why it does this, I have no idea.
> 
> Are these patches in the Debian SVN:
> 
> http://freedesktop.org/cgi-bin/viewcvs.cgi/xlibs/Xfont/fc/fserve.c?r1=3.22&r2=3.22.2.1
> http://freedesktop.org/cgi-bin/viewcvs.cgi/xlibs/Xfont/fc/fserve.c?r1=3.23&r2=3.24
> http://cvsweb.xfree86.org/cvsweb/xc/lib/font/fc/fserve.c.diff?r1=3.26&r2=3.27

No.  Fortunately all of the above predate the XFree86 1.1 relicensing.

I'm attaching a patch that should be bolted onto
debian/patches/000_stolen_from_HEAD.diff.

-- 
G. Branden Robinson                |    Damnit, we're all going to die;
Debian GNU/Linux                   |    let's die doing something *useful*!
[EMAIL PROTECTED]                 |    -- Hal Clement, on comments that
http://people.debian.org/~branden/ |       space exploration is dangerous
  3.25          +52 -2     xc/lib/font/fc/fserve.c
   603. Add font bounds checking to the X server side of the font server
        interface (Chisato Yamauchi, David Dawes).

  3.26          +18 -35    xc/lib/font/fc/fserve.c
  Combine two sets of bounds tests into one. (Chisato Yamauchi)

  3.27          +2 -2      xc/lib/font/fc/fserve.c
  Fix potential segfault.

Index: xc/lib/font/fc/fserve.c
===================================================================
RCS file: /cvs/xc/lib/font/fc/fserve.c,v
retrieving revision 3.22.2.1
retrieving revision 3.27
diff -u -r3.22.2.1 -r3.27
--- xc/lib/font/fc/fserve.c     29 Aug 2003 18:05:09 -0000      3.22.2.1
+++ xc/lib/font/fc/fserve.c     12 Jan 2004 17:19:30 -0000      3.27
@@ -24,7 +24,7 @@
 in this Software without prior written authorization from The Open Group.
 
 */
-/* $XFree86: xc/lib/font/fc/fserve.c,v 3.22.2.1 2003/08/29 18:05:09 herrb Exp 
$ */
+/* $XFree86: xc/lib/font/fc/fserve.c,v 3.27 2004/01/12 17:19:30 tsi Exp $ */
 
 /*
  * Copyright 1990 Network Computing Devices
@@ -87,13 +87,13 @@
                             (pci)->descent || \
                             (pci)->characterWidth)
 
+extern void ErrorF(const char *f, ...);
 
 static int fs_read_glyphs ( FontPathElementPtr fpe, FSBlockDataPtr blockrec );
 static int fs_read_list ( FontPathElementPtr fpe, FSBlockDataPtr blockrec );
 static int fs_read_list_info ( FontPathElementPtr fpe, 
                               FSBlockDataPtr blockrec );
 
-static int  fs_font_type;
 extern fd_set _fs_fd_mask;
 
 static void fs_block_handler ( pointer data, OSTimePtr wt, 
@@ -952,6 +952,7 @@
     CharInfoPtr                    ci, pCI;
     char                   *fsci;
     fsXCharInfo                    fscilocal;
+    FontInfoRec                    *fi = &bfont->pfont->info;
 
     rep = (fsQueryXExtents16Reply *) fs_get_reply (conn, &ret);
     if (!rep || rep->type == FS_Error)
@@ -997,6 +998,21 @@
     {
        memcpy(&fscilocal, fsci, SIZEOF(fsXCharInfo)); /* align it */
        _fs_convert_char_info(&fscilocal, &ci->metrics);
+       /* Bounds check. */
+       if (ci->metrics.ascent > fi->maxbounds.ascent)
+       {
+           ErrorF("fserve: warning: %s %s ascent (%d) > maxascent (%d)\n",
+                  fpe->name, fsd->name,
+                  ci->metrics.ascent, fi->maxbounds.ascent);
+           ci->metrics.ascent = fi->maxbounds.ascent;
+       }
+       if (ci->metrics.descent > fi->maxbounds.descent)
+       {
+           ErrorF("fserve: warning: %s %s descent (%d) > maxdescent (%d)\n",
+                  fpe->name, fsd->name,
+                  ci->metrics.descent, fi->maxbounds.descent);
+           ci->metrics.descent = fi->maxbounds.descent;
+       }
        fsci = fsci + SIZEOF(fsXCharInfo);
        /* Initialize the bits field for later glyph-caching use */
        if (NONZEROMETRICS(&ci->metrics))
@@ -1022,7 +1038,6 @@
     /* build bitmap metrics, ImageRectMax style */
     if (haveInk)
     {
-       FontInfoRec *fi = &bfont->pfont->info;
        CharInfoPtr ii;
 
        ci = fsfont->encoding;
@@ -1042,6 +1057,23 @@
            {
                ci->metrics = ii->metrics;
            }
+           /* Bounds check. */
+           if (ci->metrics.ascent > fi->maxbounds.ascent)
+           {
+               ErrorF("fserve: warning: %s %s ascent (%d) "
+                      "> maxascent (%d)\n",
+                      fpe->name, fsd->name,
+                      ci->metrics.ascent, fi->maxbounds.ascent);
+               ci->metrics.ascent = fi->maxbounds.ascent;
+           }
+           if (ci->metrics.descent > fi->maxbounds.descent)
+           {
+               ErrorF("fserve: warning: %s %s descent (%d) "
+                      "> maxdescent (%d)\n",
+                      fpe->name, fsd->name,
+                      ci->metrics.descent, fi->maxbounds.descent);
+               ci->metrics.descent = fi->maxbounds.descent;
+           }
        }
     }
     {
@@ -1498,7 +1530,6 @@
     FSBlockDataPtr         blockrec = NULL;
     FSBlockedFontPtr       bfont;
     FSFontDataPtr          fsd;
-    FSFontPtr              fsfont;
     fsOpenBitmapFontReq            openreq;
     fsQueryXInfoReq        inforeq;
     fsQueryXExtents16Req    extreq;
@@ -1522,7 +1553,6 @@
 
        font = *ppfont;
        fsd = (FSFontDataPtr)font->fpePrivate;
-       fsfont = (FSFontPtr)font->fontPrivate;
        /* This is an attempt to reopen a font.  Did the font have a
           NAME property? */
        if ((nameatom = MakeAtom("FONT", 4, 0)) != None)
@@ -1550,7 +1580,6 @@
            return AllocError;
        
        fsd = (FSFontDataPtr)font->fpePrivate;
-       fsfont = (FSFontPtr)font->fontPrivate;
     }
     
     /* make a new block record, and add it to the end of the list */
@@ -1793,7 +1822,7 @@
                            err;
     int                            nranges = 0;
     int                            ret;
-    fsRange                *ranges, *nextrange = 0;
+    fsRange                *nextrange = 0;
     unsigned long          minchar, maxchar;
 
     rep = (fsQueryXBitmaps16Reply *) fs_get_reply (conn, &ret);
@@ -1818,7 +1847,7 @@
     if (blockrec->type == FS_LOAD_GLYPHS)
     {
        nranges = bglyph->num_expected_ranges;
-       nextrange = ranges = bglyph->expected_ranges;
+       nextrange = bglyph->expected_ranges;
     }
 
     /* place the incoming glyphs */
@@ -2185,7 +2214,7 @@
        xfree(ranges);
 
        /* Now try to reopen the font. */
-       return fs_send_open_font(client, (FontPathElementPtr)0,
+       return fs_send_open_font(client, pfont->fpe,
                                 (Mask)FontReopen, (char *)0, 0,
                                 (fsBitmapFormat)0, (fsBitmapFormatMask)0,
                                 (XID)0, &pfont);
@@ -2291,7 +2320,6 @@
 {
     FSFpePtr           conn = (FSFpePtr) fpe->private;
     FSBlockDataPtr     blockrec;
-    FSBlockedListPtr   blockedlist;
     int                        err;
 
     /* see if the result is already there */
@@ -2302,7 +2330,6 @@
            err = blockrec->errcode;
            if (err == StillWorking)
                return Suspended;
-           blockedlist = (FSBlockedListPtr) blockrec->data;
            _fs_remove_block_rec(conn, blockrec);
            return err;
        }
@@ -3143,21 +3170,21 @@
 void
 fs_register_fpe_functions(void)
 {
-    fs_font_type = RegisterFPEFunctions(fs_name_check,
-                                       fs_init_fpe,
-                                       fs_free_fpe,
-                                       fs_reset_fpe,
-                                       fs_open_font,
-                                       fs_close_font,
-                                       fs_list_fonts,
-                                       fs_start_list_with_info,
-                                       fs_next_list_with_info,
-                                       (WakeupFpeFunc)fs_wakeup,
-                                       fs_client_died,
-                                       _fs_load_glyphs,
-                                       NULL,
-                                       NULL,
-                                       NULL);
+    RegisterFPEFunctions(fs_name_check,
+                        fs_init_fpe,
+                        fs_free_fpe,
+                        fs_reset_fpe,
+                        fs_open_font,
+                        fs_close_font,
+                        fs_list_fonts,
+                        fs_start_list_with_info,
+                        fs_next_list_with_info,
+                        fs_wakeup,
+                        fs_client_died,
+                        _fs_load_glyphs,
+                        NULL,
+                        NULL,
+                        NULL);
 }
 
 static int
@@ -3210,19 +3237,19 @@
 void
 check_fs_register_fpe_functions(void)
 {
-    fs_font_type = RegisterFPEFunctions(fs_name_check,
-                                       fs_init_fpe,
-                                       fs_free_fpe,
-                                       fs_reset_fpe,
-                                       check_fs_open_font,
-                                       fs_close_font,
-                                       check_fs_list_fonts,
-                                       check_fs_start_list_with_info,
-                                       check_fs_next_list_with_info,
-                                       (WakeupFpeFunc)fs_wakeup,
-                                       fs_client_died,
-                                       _fs_load_glyphs,
-                                       NULL,
-                                       NULL,
-                                       NULL);
+    RegisterFPEFunctions(fs_name_check,
+                        fs_init_fpe,
+                        fs_free_fpe,
+                        fs_reset_fpe,
+                        check_fs_open_font,
+                        fs_close_font,
+                        check_fs_list_fonts,
+                        check_fs_start_list_with_info,
+                        check_fs_next_list_with_info,
+                        fs_wakeup,
+                        fs_client_died,
+                        _fs_load_glyphs,
+                        NULL,
+                        NULL,
+                        NULL);
 }

Attachment: signature.asc
Description: Digital signature

Reply via email to