freetype-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

RE: [ft-devel] Migrating layout table validation code to a new CVS modul


From: Turner, David
Subject: RE: [ft-devel] Migrating layout table validation code to a new CVS module
Date: Fri, 18 Nov 2005 14:48:30 +0100

> > I don't know if some of you won't hate me for this, but I've
> > commited this morning a new module in the CVS named "ftvalid"
>
> Expect a bomb within a few days...
>
OK, will look for it :-)

> > IMPORTANT: Note that, at the moment, you'll need "Jam" to
> > compile the library and test program. I had no time to write
> > portable Makefiles for this project.
>
> I ask you not to remove the source files from the original location
> until the new module works with GNU make.
>
OK, but the GNU Makefiles will probably only work on Unix or Cygwin.
I'll let you handle the Autoconf/Libtool madness, I'm too old for
this.

Note: I'm not going to copy the current Make-based build system used in
FT2, it's just to clumsy and difficult to understand to deserve
this.


> BTW, I'm not really happy that you want to move it into another CVS
> module.  Those files are really part of FreeType and quite tightly
> integrated.  Instead I suggest that we extend FreeType's source tree
> structure, adding a new top-level directory `addons' which is filled
> with extra stuff.
>
First, I don't consider this code to be part of the font engine. It
does nothing for anyone trying to load and render fonts at the moment.

It can only be useful in conjunction with a higher-level text layout
library.

Apart from that, I'd be happy to put the cache sub-system in a
top-level "addons" directory. Or even to its own CVS module.

> Do you have any experience in library dependencies?  If I understand
> you correctly, otvalid and gxvalid should be a separate library
> `libftvalid' which depends on `libfreetype', right?
>
libftvalid includes "otvalid" and "gxvalid", it depends on libfreetype.
the public API if <ftvalid.h>, which depends on <ft2build.h> etc...

so it's technically a library that depends on FreeType 2, but only
uses its public API.


> >   - finally, I wonder if the code is that useful. For example, it
> >     doesn't check that all alignment constraints are respected by
> >     offsets. What this means is that even a table that was
> >     succesfully "validated" might crash a library like ICU Layout on
> >     Sparc processors.
>
> I don't really understand.  Please explain with an example.  If such
> tests are still missing we can add them.
>
As said earlier, this code can only be used in conjuction with a
higher-level text layout library that we're certain not to provide
with FT2.

The problem is that if both the layout library and validation code
are written separately, there is the possibility of a mismatch, where
what "ftvalid" think is valid might not be appropriate for the layout
one.

For example, ICU layout assumes that all 2-byte and 4-byte values are
positionned on correctly aligned addresses. However, this is not
guaranteed by the current "ftvalid" code, which doesn't check that
offsets to 2-byte values are even, or offsets to 4-byte values are
multiples of 4. (note that the spec says they should).

On a processor that doesn't support unaligned word access, this will
cause ICU layout to crash, even if "ftvalid" said the tables were
"conforming".

Another problem is that bugs in "ftvalid" can lead to problems in
the layout library. Consider for example line 125 of gxvbsln.c which
reads as:

    offset = (FT_UShort)( base_value.u +
                          ( relative_gindex * sizeof ( FT_UShort ) ) );

    p     = valid->lookuptbl_head + offset;
    limit = lookuptbl_limit;
    GXV_LIMIT_CHECK( 2 );

here, the problem is that the cast to FT_UShort loses some bits from
the value computed. If 'relative_gindex' is sufficiently big, you
could find a value for offset that is much smaller than expected,
making the computation of p fit within the table limits.

So, the table is invalid, but "ftvalid" says it's ok. when the layout
library computes the offset without a cast to a 16-bit word, like
a direct pointer operation, this will send it reading data far far
away from what "ftvalid" has checked.


In other words, there are some serious security/invalidity risks by
using distinct libraries to perform table validation and processing.

If you find a security bug in the layout library, you need to ensure
that it is also fixed in the validation code. And vice-versa... This
is a hassle.


> > I guess that we will not remove the public APIs defined so far from
> > FreeType 2, but their implementation will simply return
> > FT_Err_Unimplemented instead.
>
> Ideally, FreeType should have support for external plugins.  This is,
> if FreeType finds libftvalid, it uses it, otherwise you get
> FT_Err_Unimplemented.  It seems that we have to start with libtool's
> `dlopen' function...
>
> Sigh.  All these things mean a lot of additional work.  Isn't it
> easier to simply disable those two modules?  I'm already thinking how
> to have a central configuration file which controls the used modules
> -- without removing or renaming the rules.mk files...
>
We probably need something like that, but I want, at a minimum, that
these modules be out of a default build.

Regards,

- David
***********************************************************************************
Information contained in this email message is confidential and may be 
privileged, and is intended only for use of the individual or entity named 
above. If the reader of this message is not the intended recipient, or the 
employee or agent responsible to deliver it to the intended recipient, you are 
hereby notified that any dissemination, distribution or copying of this 
communication is strictly prohibited. If you have received this communication 
in error, please immediately notify the address@hidden and destroy the original 
message.
***********************************************************************************





reply via email to

[Prev in Thread] Current Thread [Next in Thread]