Last Comment Bug 811382 - Update OTS to r95
: Update OTS to r95
Status: VERIFIED FIXED
[adv-main18+][adv-esr17+][adv-esr10+]
: csectype-bounds, sec-high, sec-vector
Product: Core
Classification: Components
Component: Graphics (show other bugs)
: Trunk
: All All
: -- major (vote)
: mozilla19
Assigned To: Jonathan Kew (:jfkthame)
:
Mentors:
http://code.google.com/p/ots/source/d...
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-11-13 11:03 PST by Christoph Diehl [:posidron]
Modified: 2013-04-30 18:41 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
wontfix
+
verified
+
verified
18+
verified
18+
verified


Attachments
update OTS library to r.95 (8.28 KB, patch)
2012-11-15 07:19 PST, Jonathan Kew (:jfkthame)
no flags Details | Diff | Review
update OTS library to r.95 (2.89 KB, patch)
2012-11-15 07:29 PST, Jonathan Kew (:jfkthame)
jd.bugzilla: review+
akeybl: approval‑mozilla‑aurora+
akeybl: approval‑mozilla‑beta-
dveditz: approval‑mozilla‑esr10+
dveditz: approval‑mozilla‑esr17+
dveditz: sec‑approval+
Details | Diff | Review
[esr10] update OTS library to r.95. (26.86 KB, patch)
2012-11-30 02:13 PST, Jonathan Kew (:jfkthame)
akeybl: approval‑mozilla‑esr10+
Details | Diff | Review

Description Christoph Diehl [:posidron] 2012-11-13 11:03:12 PST
We should update OTS to r95 to fix an out of bounds issue.
Comment 1 Daniel Veditz [:dveditz] 2012-11-13 11:09:46 PST
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) 2012-11-15 07:19:09 PST
Created attachment 681983 [details] [diff] [review]
update OTS library to r.95
Comment 3 Jonathan Kew (:jfkthame) 2012-11-15 07:29:33 PST
Created attachment 681988 [details] [diff] [review]
update OTS library to r.95
Comment 4 Jonathan Kew (:jfkthame) 2012-11-15 16:29:34 PST
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] 2012-11-15 16:39:18 PST
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] 2012-11-15 16:48:07 PST
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] 2012-11-15 16:50:15 PST
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 9 Ed Morley [:emorley] 2012-11-16 09:54:30 PST
https://hg.mozilla.org/mozilla-central/rev/09fa9bbbfb02
Comment 10 Alex Keybl [:akeybl] 2012-11-16 14:28:12 PST
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) 2012-11-16 14:50:12 PST
https://hg.mozilla.org/releases/mozilla-aurora/rev/150e0f6c311a
Comment 12 Daniel Veditz [:dveditz] 2012-11-29 15:51:33 PST
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) 2012-11-30 00:42:25 PST
https://hg.mozilla.org/releases/mozilla-esr17/rev/dcce1bc7485a
Comment 14 Jonathan Kew (:jfkthame) 2012-11-30 00:42:37 PST
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) 2012-11-30 02:13:41 PST
Created attachment 687013 [details] [diff] [review]
[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) 2012-11-30 02:25:45 PST
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] 2012-12-04 09:23:15 PST
I'd much rather take the whole r95 update rather than try to cherry pick patches.
Comment 18 Jonathan Kew (:jfkthame) 2012-12-07 00:51:38 PST
https://hg.mozilla.org/releases/mozilla-esr10/rev/164e6a42a414
Comment 19 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2013-01-03 10:46:39 PST
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) 2013-01-03 11:56:20 PST
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 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2013-01-03 12:25:12 PST
I'll see what I can come up with. Thanks.
Comment 22 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2013-01-04 10:59:34 PST
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) 2013-01-07 05:59:38 PST
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 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2013-01-07 10:06:13 PST
(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 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2013-01-08 07:34:13 PST
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) 2013-01-08 08:06:51 PST
Sounds good to me, thanks.
Comment 27 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2013-01-08 08:44:25 PST
Marking this bug verified fixed based on comment 22 through 26.

Note You need to log in before you can comment on or make changes to this bug.