fontforge/fontforge — pull request #1798 by tshinnic (2014-10-14 15:51:46+00:00)
Title: Guard against crash on truncated/short input TTF files

Given a short TTC font file - truncated from MBs to just 150KB - routine
`PickTTFFont()` in `fontforge/parsettf.c` crashed on a NULL pointer.
This was found as a consequence of PR #1780 issue #1779.

Much input handling of TTF file data in `fontforge/parsettf.c` is not
very cautious, and assumes a valid TTF/TTC file. At a minimum some
additional checks are needed to guard against outright crashes on
short files.

Routine `TTFGetFontName()` was not checking against seeking to or
reading from areas past the end of file. This change forces return of
NULL in more error situations. This routine could be called from more
than a couple places and so needs its own separate checking.

Routine `PickTTFFont()` was not well-prepared for `TTFGetFontName()`
to return NULL instead of a font name string. If such was returned
the `offsets[]` and `names[]` arrays could get out of order, potentially
selecting the wrong font. And a NULL pointer could get handed to
`strcmp()` causing the crash.

Additionally, an index number as a font selector appended to filenames
was not checked against valid font index bounds. And some commentary
describing `PickTTFFont()` was added.

`readttfheader()` is changed to guard against use of incomplete TTF
tables. When first scanning the table entries in the TTF header, once
a table's offset and length are retrieved, these are checked against
the file's length. If out-of-bounds a message is logged and the table
info is not processed. While guarding against crashes this could
confuse users who might not recognize that large amounts of information
have gone missing. Example error message:
> Table 'glyf' extends beyond end of file and must be ignored.

Note some possible remaining problems with this change:
- substitution of string `""` on missing/invalid font name
in TTC file; is there a better substitute name? e.g. "fontname3"?
- guarding against incomplete tables only logs errors and skips tables
- no good check against too many missing tables in `readttfheader()`;
there should be a "go / no go" decision as to whether a TTF file
is simply not usable; (`ValidateTTFHead()` merely complains, but
does not prevent an invalid file against processing!)
- end of file bounds checking not done inside `PickTTFFont()`
- many many other code sections need guards/checks for safety's sake