696702 - re-enable downloadable fonts on 1.9.2

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[John Daggett (:jtd)]

Followup from bug 663668, where downloadable fonts were disabled on 1.9.2 builds under Lion, OSX 10.7:

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/0a1993a0c859

Phillipe pointed out that according to Apple release notes, 10.7.2 fixes
the underlying OSX bug:

http://support.apple.com/kb/HT5002

> ATS
> 
> Available for: Mac OS X v10.6.8, Mac OS X Server v10.6.8, OS X Lion
> v10.7 and v10.7.1, OS X Lion Server v10.7 and v10.7.1
> 
> Impact: Applications which use the ATSFontDeactivate API may be
> vulnerable to an unexpected application termination or arbitrary code
> execution
> 
> Description: A buffer overflow issue existed in the ATSFontDeactivate
> API.
> 
> CVE-ID
> 
> CVE-2011-0230 : Steven Michaud of Mozilla

I don't think we should undo the patch but rather simply restrict the use of the Lion-specific pref to 10.7 and 10.7.1 builds and have all other builds use the general pref for enabling/disabling downloadable fonts.

Comment 1

[Jonathan Kew [:jfkthame]]

Perhaps it goes without saying, but first we need to do some testing to confirm that the 10.7.2 update really does fix the problem.

Comment 2

[Steven Michaud [:smichaud] (Retired)]

I don't crash on OS X 10.7.2 in FF 3.6.18 or FF 4.0.1, testing with
either of the following URLs:

http://people.mozilla.org/~jdaggett/memtesting/iteratepages.html
http://people.mozilla.com/~stmichaud/bmo/iteratepages-663688.html

But I'm not sure if John's test currently visits any pages with
downloadable fonts.  And though I know mine did, I don't know if any
of those pages are still live.

> Followup from bug 663668, where downloadable fonts were disabled on
> 1.9.2 builds under Lion, OSX 10.7:

It's bug 663688 :-)

Comment 3

[philippe (part-time)]

(meant to post this yesterday, but apparently didn't press the submit thingie…)

I had the opportunity to run a series of test on a 10.7.2 machine [*]
* an automated pageset (steven's but with different URLs) that ran for about 40 minutes
* manually loading a whole bunch of pages with webfonts, from real world stuff to sometimes weirdo test files [.ttf, .otf, .woff and src: local()] I have on my dev server
* playing with the google fonts pages/UI
* loading a window with several tabs at once
* closing that window in one go
* quiting and restarting the browser with a window with several pages loaded

The browser [**] never crashed, I didn't notice any anomalies, the machine didn't vanish in a puff of smoke and apparently I survived. For completeness' sake, we restarted the machine at the end of the test.

[*] ~1year old MBP with 4gig of ram, Adobe CS5 fonts installed, new user account
[**] Gecko 1.9.2 obviously…

Comment 4

[Jonathan Kew [:jfkthame]]

And (just to confirm), was the gfx.downloadable_fonts.enabled.lion pref set to TRUE for all this testing?

Comment 5

[philippe (part-time)]

(In reply to Jonathan Kew (:jfkthame) from comment #4)
> And (just to confirm), was the gfx.downloadable_fonts.enabled.lion pref set
> to TRUE for all this testing?

Of course - that is the point, right? And before you ask, all requested fonts loaded (it is hard to escape the beauty of src:local(ahem)...).

Comment 6

[Jonathan Kew [:jfkthame]]

Attachment: patch, only use the lion-specific pref on 10.7.0 - 10.7.1

OK, this should implement John's suggested behavior of using the lion-specific pref only on the buggy releases, and reverting to the generic one on 10.7.2.

(It's a pity that we'll then have a pref that sounds like it ought to affect current Lion systems but is in fact ignored. I suppose we could consider changing its name to something more explicit - and very long-winded - like "gfx.downloadable_fonts.enabled.lion-10.7.0-10.7.1" but I'm not sure that is really worthwhile.)

Comment 7

[Steven Michaud [:smichaud] (Retired)]

> And (just to confirm), was the gfx.downloadable_fonts.enabled.lion pref set
> to TRUE for all this testing?

Yes.

Comment 8

[Jonathan Kew [:jfkthame]]

Comment on attachment 569628 [details] [diff] [review]
patch, only use the lion-specific pref on 10.7.0 - 10.7.1

We disabled downloadable fonts on OS X 10.7 due to an Apple bug, but now that the OS bug is fixed, we'd like to re-enable the feature for users on the up-to-date Lion release.

Comment 9

[Daniel Veditz [:dveditz]]

Comment on attachment 569628 [details] [diff] [review]
patch, only use the lion-specific pref on 10.7.0 - 10.7.1

Unfortunately we're a couple of weeks past code-freeze, we should try getting this into the next release.

Comment 10

[Daniel Veditz [:dveditz]]

Comment on attachment 569628 [details] [diff] [review]
patch, only use the lion-specific pref on 10.7.0 - 10.7.1

Approved for 1.9.2.25, a=dveditz

Does this bug need to be hidden? The scary crash was fixed many releases ago and this is just re-enabling a feature.

Comment 11

[Jonathan Kew [:jfkthame]]

https://hg.mozilla.org/releases/mozilla-1.9.2/rev/7a0309c9c7e7

Fixed for 1.9.2.25, but the tracking flags don't offer that value yet.

Un-hiding this, as suggested in comment 10. The original crash (bug 663688 - note that the bug number was typo'd in comment 0 and in the commit message of cset 0a1993a0c859) has been unhidden already.

Comment 12

[Mark Banner (:standard8)]

(In reply to Jonathan Kew (:jfkthame) from comment #11)
> https://hg.mozilla.org/releases/mozilla-1.9.2/rev/7a0309c9c7e7
> 
> Fixed for 1.9.2.25, but the tracking flags don't offer that value yet.

It is present now, so setting that flag.

Comment 13

[Al Billings [:abillings - ex-MoCo]]

I've verified this in the nightly 1.9.2 build (Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.7; en-US; rv:1.9.2.25pre) Gecko/20111209 Namoroka/3.6.25pre) using:

http://people.mozilla.org/~jdaggett/memtesting/iteratepages.html
http://people.mozilla.com/~stmichaud/bmo/iteratepages-663688.html

and letting it run a while with the gfx.downloadable_fonts.enabled.lion pref set to TRUE in the profile.

No crashes so this appears to be fixed. (This is on the current 10.7 version.)