2.89 KB, patch
|Details | Diff | Splinter Review|
26.86 KB, patch
|Details | Diff | Splinter Review|
We should update OTS to r95 to fix an out of bounds issue.
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.
Created attachment 681983 [details] [diff] [review] update OTS library to r.95
Created attachment 681988 [details] [diff] [review] update OTS library to r.95
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 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.
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 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 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 on attachment 681988 [details] [diff] [review] update OTS library to r.95 Approved for ESR-10 and ESR-17 branches.
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.)
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 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.
I'd much rather take the whole r95 update rather than try to cherry pick patches.
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?
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.
I'll see what I can come up with. Thanks.
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
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.
(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.
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.
Sounds good to me, thanks.
Marking this bug verified fixed based on comment 22 through 26.