Re: [ft-devel] [Fontconfig] fontconfig crash for special bdf font

2014-01-28 Thread Akira TAGOH
[Cc'ing freetype-devel and Werner]

That prop.u.atom is the result of calling FT_BDF_Get_Property though,
we are expecting to see the proper atom when prop.type is set to
BDF_PROPERTY_TYPE_ATOM. in this case IMHO prop.type should be set to
BDF_PROPERTY_TYPE_NONE and returns an error.

Werner, any comments for that?

On Tue, Jan 28, 2014 at 5:48 PM, Petr Gajdos  wrote:
> Hello,
>
> a crash in libfontconfig was reported to me. Run
>
> $ fc-query startchar.bdf
>
> (startchar.bdf is reproducer for buffer overflow
> in libXfont, see [1])
>
> The problem is following:
>
> Breakpoint 3, IA__FcFreeTypeQueryFace (face=0x608dd0,
> file=0x7fffebb9 "startchar.bdf", id=0, blanks=0x0) at
> fcfreetype.c:1591
> 1591width = FcIsWidth ((FcChar8 *) prop.u.atom);
> (gdb) p prop.u.atom
> $6 = 0x0
>
> Following patch fixes problem for me, but maybe this is not correct
> place for this check.
>
> Index: src/fcstr.c
> ===
> --- src/fcstr.c.orig2013-10-11 05:10:18.0 +0200
> +++ src/fcstr.c 2014-01-28 09:34:05.409800632 +0100
> @@ -26,6 +26,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #ifdef HAVE_REGEX_H
>  #include 
>  #endif
> @@ -211,6 +212,7 @@
>  FcChar8c1, c2;
>
>  if (s1 == s2) return 0;
> +if (!s1 || !s2) return INT_MAX;
>
>  FcStrCaseWalkerInit (s1, &w1);
>  FcStrCaseWalkerInit (s2, &w2);
>
> Petr
>
> [1]
> http://cgit.freedesktop.org/xorg/lib/libXfont/commit/?id=4d024ac10f964f6bd372ae0dd14f02772a6e5f63
>
>
> ___
> Fontconfig mailing list
> fontcon...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/fontconfig
>



-- 
Akira TAGOH

___
Freetype-devel mailing list
Freetype-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/freetype-devel


Re: [ft-devel] [Fontconfig] fontconfig crash for special bdf font

2014-01-28 Thread Werner LEMBERG

> That prop.u.atom is the result of calling FT_BDF_Get_Property
> though, we are expecting to see the proper atom when prop.type is
> set to BDF_PROPERTY_TYPE_ATOM.  In this case IMHO prop.type should
> be set to BDF_PROPERTY_TYPE_NONE and returns an error.

Why?  The entry in the font causing the crash is is

  SETWIDTH_NAME ""

which is valid from a syntax point of view AFAICS, so returning an
error seems incorrect to me.

However: There is explicit code in FreeType's BDF handling which
prevents the creation of an empty string (cf. bdflib.c, line 1394),
thus NULL is a valid value for `prop.u.atom'.  Looking up the code I
see that this is a change from 2007, and before that change FreeType
indeed returned an empty string...

I see two possibilities on the FreeType side:

  1. Document that NULL is a valid return value for `atom',
 corresponding to a property present in the font with a zero
 string as an argument – the return value
 `FT_Err_Invalid_Argument' of function `FT_Get_BDF_Property'
 indicates that the searched property isn't present in the font.

  2. Change the code back to return an empty string.

I tend to solution 1, but I'm open to other suggestions.


Werner
___
Freetype-devel mailing list
Freetype-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/freetype-devel