Closed Bug 483459 Opened 15 years ago Closed 15 years ago

Gecko doesn't load font files with bad checksums

Categories

(Core :: Graphics, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: dbaron, Assigned: jtd)

References

Details

(Keywords: fixed1.9.1)

Attachments

(2 files)

This was initially reported in bug 70132 comment 173 (see from there to bug 70132 comment 182).

Currently gfxFontUtils::ValidateSFNTHeaders checks that the font checksum is correct.  I've run into a decent number of fonts that fail this test, and that we therefore don't load.

One testcase is (from the comments I mentioned two paragraphs up) http://klebom.se/stellan/css/font-face/vera2.html in which  Bitstream Vera Sans Bold, Bitstream Vera Sans Oblique, Bitstream Vera Serif, and Bitstream Vera Serif Bold fail to load.  (Or maybe it's not exactly that set; I tried modifying the testcase locally to rename the families to avoid collisions with locally installed ones, and I got confusing results.  Though I think Serif Regular is also a problem.)

I also noticed that both regular and italic monofur from http://www.eurofurence.net/monofur.html fail to load.


I can see two possible causes:
 (1) there's something wrong with our checksum computation code that causes it to fail in some cases
 (2) it's common for fonts to have bad checksums since other systems that use fonts don't check them.

I suspect (2) is more likely given that the four Bitstream Vera fonts with problems have a header claiming the checksum is 0xFFF00000 (unless we're getting *that* wrong).
Hrm.. I think this should be a wontfix and a relnote.  We may want to have a pointer to some freely available tool to recalculate the checksums and/or inform the user if they're wrong.  (Assuming we're not pulling out the header incorrectly :)
Will there be some way a user, who may have nothing to do with the web site which is publishing the bad font, to know that this is what is causing the problem?

It makes sense for Firefox to DTRT but it would be good if innocent users were not punished for the mistakes of font publishers.

Will using a font that seems to have a bad checksum actually cause a problem? Or is it just that something _might_ go wrong?
We already spam the error console with lots of stuff, so adding a "User font foo.ttf not loaded because it contains errors" message sounds fine.
Before loading a font we perform some simple sanity checks in gfxFontUtils::ValidateSFNTHeaders.  One of them is a checksum check, which simply validates that the sum of the table header checksums matches the checksum in the 'head' table.  I'm fairly sure this calculation is correct, it's not that complex and no fonts would load if it wasn't correct.

The validation code also checks to make sure that the table directory is sensible, that none of the offset/length values point to data past the end of font data.  This will catch cases where font data is obviously corrupt (e.g. accidentally using a random data file as a font).

If the problem is just a bad checksum, it's fairly easy to correct.  Simply dumping the font out and recompiling it with ttx (http://fonttools.sourceforge.net/) should do the trick.  I'd much rather authors be forced to publish copasetic data rather than allowing funky fonts through, even if those fonts seem to work just fine in most cases.  If we were in full control of the font rasterization on all platforms, it might make sense to force better error handling downstream but we've already seen several Apple problems on the Mac that result from insufficient data checking (see bug 458861 for an egregious example).  We can push to get these problems fixed but that still leaves users exposed to problems on older platforms.

I agree that posting an error in the console on a font load failure sounds like a good idea.
Proposed change: nsFontDownloader should log font load errors to console via nsIConsoleService mechanism with details of load failure (font error, invalid cross-domain access, network error, etc.).

One note: any console error would need to use localizable strings (ideally), so adding late-l10n keyword.  String freeze for beta4 is this week (3/19).  I'll create separate patches for string change and actual code change that uses strings.
Assignee: nobody → jdaggett
Keywords: late-l10n
Sounds good per se, I have some thoughts that are applicable here.

When adding messages that target a highly technical audience about, like here, webstandards with scarce l10n of the documentation, English error messages might be the right choice for localization.

Thus, for mozilla-central, I'd love to get a localization note indicating that.

LOCALIZATION NOTE (foo.bar): This message might be more suitable in English, depending on available documentation of downloadable fonts in your language.

(assuming this is downloadable fonts)

For 1.9.1, we might want to take a patch with hardcoded strings, or with the localizable file in the content jar so that it's not part of l10n. But we should make that call once we have a patch on central and this bug is triaged to be blocking 3.5.
If you want this localized, you'll need the strings patch in today. I thought, though, fwiw, that we have a lot of error console spew that's not localized, so I don't think that's a big deal.
Today I pulled down most of the fonts on www.fontsquirrel.com, a site that highlights interesting free fonts.  Of the 700 fonts I tested, roughly 4% failed to load due to bad checksums.  Under Mac OS X, FontBook never complains about bad checksums when validating fonts.  Bad checksums are easy to fix, dumping the font data using ttx and recompiling it will result in a font with a correct checksum but I'm not sure requiring this helps much.

So I'm thinking it may make sense to relax this.  The existing code is not actually calculating a full checksum for the entire font data, it's only verifying that the checksum for all the tables stored in the table directory matches up with the overall checksum stored in the head table, so it's only going to catch data corruption errors that affect the table directory.  I think carefully verifying offset values in the table directory should be enough.
It'd be interesting to do some further validation of those "bad" fonts, to see whether incorrect checksums are the only problems. Could you provide a list of the failures, to save re-checking the whole collection?

My guess is that if the checksum fails, this doesn't usually indicate data corruption (ie, bad download, etc) but rather is a clue that the font was created or modified with low-quality tools. As such, refusing to load the font may be the most prudent course, given the potential for crashing if fonts are poorly constructed in other respects.

Of course, a genuine maliciously-crafted font would probably have valid checksums, so this won't protect us from a deliberate attack. But it might well reduce the risk of crashing due to fonts that are merely incorrect rather than evil.
These fonts have bad checksums:

Amaze/amaze-bold.ttf
Amaze/amaze-italic.ttf
Amaze/amaze.ttf
Aquiline/Aquiline.ttf
BeesWax/BEESWAX.TTF
Bitstream-Vera-Sans/Vera-Bold.ttf
Bitstream-Vera-Sans/Vera-Italic.ttf
Bitstream-Vera-Serif/VeraSerif-Bold.ttf
Bitstream-Vera-Serif/VeraSerif.ttf
Boycott/BOYCOTT_.TTF
Destroy/DESTROY_.TTF
Diesel/Truetype/DIESEL__.TTF
DingMaps/DINGMAPS.TTF
Downcome/Truetype/DOWNCOME.TTF
DsgstngBhvr/Truetype/DISGB___.TTF
Enigmatic/Enigma_2i.TTF
Enigmatic/Enigma__2.TTF
Enigmatic/EnigmaB_2.TTF
Guilty/Truetype/GUILTY__.TTF
Hatcheck/HATCHECK.TTF
Heavyweight/HEAVYWEI.TTF
Intruder-Alert/Intruder.ttf
Jinky/JINKY.TTF
Mirisch/MIRISCH.TTF
Misproject/Truetype/MISPROJE.TTF
NailScratch/Truetype/NAILS___.TTF
Nasty/Truetype/NASTY___.TTF
Pisan/PISAN.TTF
QuillScript/Quill_script.ttf
Romanche/ROMANCHE.TTF
Shortcut/Truetype/SHORTCUT.TTF
Silkscreen/slkscr.ttf
Silkscreen/slkscrb.ttf
Silkscreen/slkscre.ttf
Silkscreen/slkscreb.ttf
Sjonarbok-Classic/sjonarbok_classic.ttf
Water-Street/waterst.ttf
Water-Street/waterst2.ttf

All were downloaded from the fontsquirrel site.
Im not sure that TTX adds a valid DSIG.  

TrueType / OpenType fonts optionally may contain a DSIG. See: <http://www.microsoft.com/Typography/otspec/dsig.htm>.  Microsoft distribute an OpenType Font Signing Tool for Windows to add  DSIGs to fonts  see: <http://www.microsoft.com/typography/developers/dsig/dsig.aspx> - but to do this it seems you need an identity certificate and encryption keys from a certifying agency, such as Verisign. Once signed any change to the font file invalidates the DSIG. Consequently very few FOSS fonts - unless thy were originally produced by a commercial vendor like Bitstream or Ascender - contain a DSIG. If a FOSS font containing a DSIG is modified either the font should be re-signed with a new DSIG - or the DSIG table should be removed since otherwise any app checking the DSIG will find a bad checksum and flag it as invalid. This is something anyone modifying free/libre fonts needs to watch out for.

(A font with an invalid DSIG may indicate that it was originally a font from commercial vendor that someone has opened in a font editor, modified and/or renamed. Or it may indicate that someone used a font from a commercial vendor as a template while creating the font.)      

-C
I did a quick test on a bunch of fonts pulled down from free font sites.

5463 fonts
 471 fails (467 bad checksum, 3 ATS load error, 1 table directory entry)

MgOpen fonts
http://www.ellak.gr/fonts/mgopen/index.en.html

FontSquirrel
http://www.fontsquirrel.com/

Greek Font Society
http://www.greekfontsociety.gr/

Dieter Steffmann
http://www.moorstation.org/typoasis/designers/steffmann/

Manfred Klein
http://moorstation.org/typoasis/designers/klein/

Dr Berlin
http://moorstation.org/typoasis/designers/DrBerlin/user.dtcc.edu/_berlin/fonts.html

Many of the bad checksum fonts were in the Dr Berlin font set, these seem to be fairly old.
Let's please log these errors; it will make debugging web content much easier.
Keeps the existing checksum validation but doesn't reject the font based on it.  This will allow Vera fonts with bad checksums to be used.
Attachment #378271 - Flags: review?(vladimir)
Comment on attachment 378271 [details] [diff] [review]
patch, v.0.1, warn about bad checksum but don't block font

Do we want to warn in the error console?  (That can be a separate patch if we do)
Attachment #378271 - Flags: review?(vladimir) → review+
Pushed to trunk
http://hg.mozilla.org/mozilla-central/rev/7ec819137957

Yes, we should provide console errors for these.  I'm going to do that in a separate patch.
Keywords: late-l10n
Attachment #378271 - Flags: approval1.9.1?
Attachment #378271 - Flags: approval1.9.1? → approval1.9.1+
Pushed reftest to mozcentral
http://hg.mozilla.org/mozilla-central/rev/0f869f44c740

Pushed to 1.9.1
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/52e306e00703

Note: version of VeraBd.ttf with valid checksum generated by recompiling font with ttx (http://fonttools.sourceforge.net):

  ttx VeraBd.ttf
  mv VeraBd.ttx VeraBd-validchecksum.ttx
  ttx VeraBd-validchecksum.ttx

This produces VeraBd-validchecksum.ttf.  This utility is extremely handy for doing quick, on-the-fly modifications of font data.
Status: NEW → RESOLVED
Closed: 15 years ago
Keywords: fixed1.9.1
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: