811382 - Update OTS to r95

Status: VERIFIED FIXED

(Core :: Graphics, defect)

Description

[Christoph Diehl [:posidron]]

We should update OTS to r95 to fix an out of bounds issue.

Comment 1

[Daniel Veditz [:dveditz]]

some information at https://chromiumcodereview.appspot.com/10913058, the chromium bug is hidden.
http://code.google.com/p/chromium/issues/detail?id=146254

Also identified as CVE-2012-2897, and MS is apparently fixing the underlying kernel bug today https://twitter.com/NTarakanov/status/267912298776104962

There may still be other OS bugs triggerable from the same table, we should still take the OTS patch.

Comment 2

[Jonathan Kew [:jfkthame]]

Attachment: update OTS library to r.95

Comment 3

[Jonathan Kew [:jfkthame]]

Attachment: update OTS library to r.95

Comment 4

[Jonathan Kew [:jfkthame]]

Comment on attachment 681988 [details] [diff] [review]
update OTS library to r.95

[Security approval request comment]
How easily can the security issue be deduced from the patch?
The flaw in OTS is easy to see; it's harder to know what its actual impact would be, or how to exploit the loophole, as the real problem is within Windows code; this patch is about preventing the bad data being passed on to Windows in the first place.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
Google has opened up the corresponding Chromium bug, which has details of the problem.

Which older supported branches are affected by this flaw?
All

If not all supported branches, which bug introduced the flaw?

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
Trivial to backport, negligible risk.

How likely is this patch to cause regressions; how much testing does it need?
This patch may cause us to reject a webfont that is currently accepted, which could appear as a "regression" to an author relying on a broken font; but that's its intended effect.

Given that the upstream OTS/chromium reports and patch are public, and given the simplicity of the patch, I think we should land this on all branches ASAP.

Comment 5

[Alex Keybl [:akeybl]]

Comment on attachment 681988 [details] [diff] [review]
update OTS library to r.95

[Triage Comment]
Too late in the cycle (one day before RC) to take this into FF17.

Comment 6

[Daniel Veditz [:dveditz]]

Now that we can see Chrome's bug it looks like the specific OS crash is not triggerable in Firefox because we use different windows drawing calls. A variant font still might be able to tickle the OS in untoward ways but as of right now we don't know of one.

Comment 7

[Daniel Veditz [:dveditz]]

Comment on attachment 681988 [details] [diff] [review]
update OTS library to r.95

sec-approval+

Since Chrome's patch was already public there's no need to delay landing on mozilla-central. Other branch landings still require release-driver approval.

Comment 8

[Jonathan Kew [:jfkthame]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/09fa9bbbfb02

Comment 9

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/09fa9bbbfb02

Comment 10

[Alex Keybl [:akeybl]]

Comment on attachment 681988 [details] [diff] [review]
update OTS library to r.95

We'll approve for the ESRs after we ship this next round on Tuesday. Please land on Aurora now, however, to make the merge.

Comment 11

[Jonathan Kew [:jfkthame]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/150e0f6c311a

Comment 12

[Daniel Veditz [:dveditz]]

Comment on attachment 681988 [details] [diff] [review]
update OTS library to r.95

Approved for ESR-10 and ESR-17 branches.

Comment 13

[Jonathan Kew [:jfkthame]]

https://hg.mozilla.org/releases/mozilla-esr17/rev/dcce1bc7485a

Comment 14

[Jonathan Kew [:jfkthame]]

I notice that ESR-10 is currently at a significantly older version of OTS; i.e. we haven't been backporting OTS updates as we've been taking them over the past year. Do we want to just cherry-pick this fix (it applies cleanly to the older version, so we can do that without difficulty)? Or if we're going to touch OTS in ESR-10, should we simply take the complete update from r.74 to r.95, so as to pick up other correctness fixes that have happened in the meantime?

(Personally, I think it would make most sense to take the full update. There are a few other revs listed at http://code.google.com/p/ots/source/list that look like they could have possible security implications, and I don't think there's any significant stability risk in taking the update.)

Comment 15

[Jonathan Kew [:jfkthame]]

Attachment: [esr10] update OTS library to r.95.

For ESR-10 consideration: this is the patch to update OTS to r.95 from the version currently in ESR-10 (r.74), rather than only cherry-picking the couple of most recent changes. (See http://code.google.com/p/ots/source/list for the upstream changelog.)

Comment 16

[Jonathan Kew [:jfkthame]]

Comment on attachment 687013 [details] [diff] [review]
[esr10] update OTS library to r.95.

Marking approval-mozilla-esr10? for consideration as per comment 14. In effect, this rolls up the fixes we took in bug 712217, bug 730190, bug 747816 and bug 762484, together with the most recent fixes in this bug.

The code changes are pretty minor, but include at least one potential OOB issue as well as a few sanitization correctness fixes, so although the earlier bugs were not specifically called out as security issues, I think it would make sense to take the complete update rather than cherry-picking.

If this is -not- accepted for ESR-10, we can still land the smaller patch that only cherry-picks the latest two fixes but leaves earlier issues unaddressed.

Comment 17

[Daniel Veditz [:dveditz]]

I'd much rather take the whole r95 update rather than try to cherry pick patches.

Comment 18

[Jonathan Kew [:jfkthame]]

https://hg.mozilla.org/releases/mozilla-esr10/rev/164e6a42a414

Comment 19

[u279076]

Is there any type of testing QA can do for the upcoming releases (18, 10.0.12esr, 17.0.2esr) to shake out potential regressions?

Comment 20

[Jonathan Kew [:jfkthame]]

All downloadable webfonts are processed by OTS, so regressions would most likely show up as webfont problems (fonts failing to load, or in the worst case, crashes when trying to load them). So it'd be good to have lots of testing on sites that use a variety of webfonts, I guess.

Comment 21

[u279076]

I'll see what I can come up with. Thanks.

Comment 22

[u279076]

Jonathan, I had Softvision do some dogfooding of Web Fonts in 10.0.2esr candidate builds overnight and this is what they came up with:
https://etherpad.mozilla.org/Web-Fonts-Exploratory

Can you please review their results and let me know if this meets verification of this bug? If not, please advise what we can test further.

Thank you

Comment 23

[Jonathan Kew [:jfkthame]]

It looks like many of the "scenarios" described there involve downloading and installing a font locally, and then using it in the browser; IOW those aren't web-font (CSS @font-face) tests but general font-installation and font-selection tests.

The scenarios that use the Google webfonts service are indeed testing OTS; for more extensive testing, maybe it'd be worth checking examples that use other webfonts services as well, such as http://fontdeck.com, http://typekit.com, and http://fontfonter.com; or use fonts from http://fontsquirrel.com via @font-face kits rather than downloading the font and installing locally.

Comment 24

[u279076]

(In reply to Jonathan Kew (:jfkthame) from comment #23)
> maybe it'd be worth checking examples that use
> other webfonts services as well

Sure, I'll have them check a few of these tonight.

Comment 25

[u279076]

Jonathan, Softvision tested with the same scenarios as before using the font sites you suggested and did not find any regressions. Unless there is something else you'd like tested, I think we can call this verified fixed and keep our eye out for potential real-world feedback.

Comment 26

[Jonathan Kew [:jfkthame]]

Sounds good to me, thanks.

Comment 27

[u279076]

Marking this bug verified fixed based on comment 22 through 26.