487549 - Crash [@ ProcessKerningRun] with lots of web fonts

Status: VERIFIED FIXED

(Core :: Graphics, defect, P2)

Description

[Jesse Ruderman]

Attachment: stack trace

Loading http://fonts.philip.html5.org/ triggers a scary crash [@ ProcessKerningRun].  Using Firefox trunk (debug) on Mac OS X 10.5.x.

Comment 1

[John Daggett (:jtd)]

The crash occurs within InitMetrics, in a call to gfxAtsuiFont::GetCharWidth.

Comment 2

[Jonathan Kew [:jfkthame]]

This is caused by a buggy .ttf font, possibly a result of the experimental subsetting tool being used to generate the files referenced by that page.

It crashes for me on the PixAntiqua face. I downloaded the .ttf file and ran Apple's ftxvalidator tool, which immediately reports problems:

 ====== /Users/jonathan/Downloads/4f614b05599ce06f164da5113070e53f.ttf ============================================
 ------ sfnt: 84d0e25f5bea105a - subset of PixAntiqua, sfnt ID: -23481 ------
    Error: 'cmap':  Format 4 final segment has <0xFFFF,0xFFFF> range but idDelta != 1
    Error: 'cmap':  Format 4 final segment has <0xFFFF,0xFFFF> range but idDelta != 1
    Error: maxp: maximum contour count should be 3.
    Error: bad bounding box entry xMin, expected: 0, actual: -171
    Error: bad bounding box entry xMax, expected: 597, actual: 853
    Error:   advanceMax (938) is wrong, it should be 682
    Error:   sideBearingMin (-171) is wrong, it should be 0
    Error:   extentMax (853) is wrong, it should be 597
    Error: bad kerning subTable count 0xC0000003
    Error: Postscript name (index 39) has non-ASCII character(s)
    Error: Name index (40) bad 2-byte character in name
    Error: multiple postscript names: 2

Not all of these are significant, but the kern subtable count certainly looks alarming (and note that our crash is happening within ATSUI's kerning processing). Trying to dump the 'kern' table itself using ftxdumperfuser, which would interpret the table and show its contents, gives a segfault. Hex-dumping the file, however, reveals that the TrueType header lists the 'kern' table as being just 4 bytes long.

This would be OK (although empty) if it conformed to the old Microsoft spec, with those 4 bytes being a USHORT version field with value = 0, and a USHORT subtable count of zero. But what is actually there is 0x00010000, which means this is supposedly a newer format 1.0 (Mac-only) table with 32-bit header fields, and its subtable count is completely missing (so ATSUI reads off the end of the table).

BTW, using the Core Text rendering path, this page displays fine; clearly, CT is more robust in the face of this particular error.

Comment 3

[Philip Taylor]

The kern table version is 1, but the Font::TTF library only understands version 0, which results in the invalid output. I've changed my code to just drop any non-0 kern tables, so http://fonts.philip.html5.org/ should hopefully work better now. (Doesn't fix the possible underlying security vulnerability, though.)

Comment 4

[Jonathan Kew [:jfkthame]]

Thanks, Philip. Yes, the page displays fine via ATSUI now.

I've kept a copy of the buggy subsetted font, so if we want to add validation to catch this, we can still test it against that.

The fundamental issue, though, is that the operating system's font service is not fully robust when presented with bad font data, and it is not feasible to exhaustively validate every font before trying to use it. We can do some sanity-checking but the final responsibility for a crash like this within ATSUI must be Apple's.

Comment 5

[Jonathan Kew [:jfkthame]]

Reported to Apple as rdar://6773700.

Comment 6

[Jonathan Kew [:jfkthame]]

Also reported to the author of the Font::TTF code that created the invalid font.

Comment 7

[Jonathan Kew [:jfkthame]]

Attachment: add some basic checking of the kern table header, and a crashtest for this

The validation here is nowhere near exhaustive (that's essentially impossible, given the complexity of font structures), but at least it provides some basic sanity-checking for the 'kern' table header, and protects us (or rather ATSUI) from basic errors such as the bad 'kern' table version in Philip's font.

Crashtest is included; this fails without the patch, and correctly refuses to use the font (and logs a warning) with the patch applied.

Comment 8

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Can we get this to m-c today, so that we can bake it and get it into 3.5b4?  Would be good to see if there are other ATSUI/webfont tickles out there, so we'd want to not have this mask them in the beta.

Comment 9

[John Daggett (:jtd)]

Pushed to mozilla-central
http://hg.mozilla.org/mozilla-central/rev/7a3d372ea6de

Comment 10

[John Daggett (:jtd)]

Pushed to mozilla 1.9.1
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/7cc518842535

Comment 11

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Same crash on Windows XP.

Verified fixed on trunk and 1.9.1 with:

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.2a1pre) Gecko/20090421 Minefield/3.6a1pre ID:20090421041537

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1b4pre) Gecko/20090421 Shiretoko/3.5b4pre ID:20090421042301

Comment 12

[Daniel Veditz [:dveditz]]

Verifying this was pushed to the 1.9.2 branch
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/7a3d372ea6de