Re: Firefox 44 (and Chrome in future) may reject some web fonts that were previously accepted

[+Khaled]

On Tue, Jan 5, 2016 at 4:03 PM, Roderick Sheeter <rsheeter@google.com>
wrote:

> Could we fix the OTS checks and have consistency and happiness once again?
>

It's a lot of work.  Would require auditing all of the GSUB/GDEF/GPOS
code.  It's not worth my time.  If someone else wants to do, they are
welcome to.  And most of the checks we are talking about are bogus.

Here, I just opened ots/src/gsub.cc and spent 20 seconds on it.
In ParseReverseChainingContextSingleSubstitution(), there's this gem of a
piece of code:


https://github.com/khaledhosny/ots/blob/5424cdf49c8a2cc0c2dec7dcd44f745be0032dfd/src/gsub.cc#L425

"""
  const uint16_t num_glyphs = font->maxp->num_glyphs;

  uint16_t backtrack_glyph_count = 0;
  if (!subtable.ReadU16(&backtrack_glyph_count)) {
    return OTS_FAILURE_MSG("Failed to read backtrack glyph count in reverse
chaining table");
  }
  if (backtrack_glyph_count > num_glyphs) {
    return OTS_FAILURE_MSG("Bad backtrack glyph count of %d",
backtrack_glyph_count);
  }
"""

That's code for validating this lookup:

  https://www.microsoft.com/typography/otspec/gsub.htm#RCCS

Let me explain what it's doing:

  - num_glyphs is the number of glyphs in the font.  For a subset font that
has, say, only 'f', 'i', and a couple of ligatures of those, it might be 5,

  - backtrack_glyph_count is the length of the lookback context string in
the lookup.  Eg, if the lookup is matching something that should only
happen when preceded by "ffffffffffff", then backtrack_glyph_count would be
13.

If backtrack_glyph_count > num_glyphs in
a ParseReverseChainingContextSingleSubstitution(), then ots rejects the
gsub table.  Go figure what the author was thinking.  It's clear as day, to
anyone knowing OpenType, that whoever wrote this code had no clue how
OpenType substitution and positioning work.  But because there's the word
"security" attached to OTS, people ignore these.

Now, that's a extremely unlikely font to come across.  But fortunately,
when someone does, it would be on Jonathan to debug and file the ots bug
for it, not me, because we don't use this code in Chrome.  It's an insult,
in fact, to HarfBuzz code, that this code has to gatekeep what reaches it.

For the reasons sampled above, I don't think GSUB/GPOS/GDEF stuff in OTS is
worth fixing, nor is needed when those tables go to HarfBuzz directly.  I
can probably uncover one bug per every minute I spend reading that code.

behdad





> On Tue, Jan 5, 2016 at 4:36 AM, Behdad Esfahbod <behdad@google.com> wrote:
>
>> On Tue, Jan 5, 2016 at 10:10 AM, Jonathan Kew <jfkthame@gmail.com> wrote:
>>
>>> On 4/1/16 22:54, Roderick Sheeter wrote:
>>>
>>>> Just a quick heads up, Firefox 44, coming Jan 26 2016
>>>> (https://developer.mozilla.org/en-US/Firefox/Releases/44) updates OTS
>>>> to
>>>> reject fonts if it rejected any of { GDEF, GSUB, GPOS }. Chrome will
>>>> pick this up at some point as well.
>>>>
>>>>
>>> I'm not sure this will affect Chrome, actually; my understanding is that
>>> Blink now lets GDEF/GSUB/GPOS tables bypass OTS validation, on the grounds
>>> that harfbuzz does its own sanitization before using them and therefore
>>> should be safe from malformed/malicious tables.
>>>
>>> (See https://codereview.chromium.org/1306343006/)
>>>
>>> It's possible we'll do something like that in Gecko at some point,
>>> though in principle I'd prefer to see pressure brought to bear on
>>> designers/authors to get incorrectly-built fonts fixed.
>>
>>
>> I fully agree with the latter statement.  Though, the reason we dropped
>> OTS check was that they were too limiting, rejecting perfectly legitimate
>> tables.
>>
>>>
>>> JK
>>>
>>>
>>> Previously OTS would drop the table(s) but accept the font so it would
>>>> work as a web font, albeit potentially with odd behavior due to the
>>>> missing tables.
>>>>
>>>> This change causes some fonts that were previously accepted to be
>>>> rejected by the browser. If so, those fonts will require updates to
>>>> continue to work as web fonts.
>>>>
>>>> See https://github.com/khaledhosny/ots/issues/74 for additional context
>>>> around the OTS change.
>>>>
>>>> Rod S
>>>>
>>>
>>>
>>>
>>
>