Last Comment Bug 753623 - (CVE-2012-3971) Graphite 2 crash [@graphite2::Pass::readPass]
(CVE-2012-3971)
: Graphite 2 crash [@graphite2::Pass::readPass]
Status: VERIFIED FIXED
[asan][sg:high][advisory-tracking+]
: crash, sec-high, testcase
Product: Core
Classification: Components
Component: Graphics (show other bugs)
: Trunk
: x86_64 Mac OS X
: -- critical (vote)
: mozilla15
Assigned To: Jonathan Kew (:jfkthame)
:
: Milan Sreckovic [:milan]
Mentors:
Depends on:
Blocks: fuzzing-fonts
  Show dependency treegraph
 
Reported: 2012-05-09 18:56 PDT by Christoph Diehl [:posidron]
Modified: 2012-10-25 17:57 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+
affected
+
fixed
+
fixed
unaffected


Attachments
testcase (31.94 KB, application/zip)
2012-05-09 18:56 PDT, Christoph Diehl [:posidron]
no flags Details
callstack (5.78 KB, text/plain)
2012-05-09 18:56 PDT, Christoph Diehl [:posidron]
no flags Details
fresh-callstack (8.18 KB, text/plain)
2012-05-29 13:29 PDT, Christoph Diehl [:posidron]
no flags Details
patch from upstream (1.99 KB, patch)
2012-05-30 03:11 PDT, Jonathan Kew (:jfkthame)
no flags Details | Diff | Splinter Review
update graphite2 to release 1.1.3 (7.37 KB, patch)
2012-05-30 04:01 PDT, Jonathan Kew (:jfkthame)
jd.bugzilla: review+
cdiehl: feedback+
Details | Diff | Splinter Review

Description Christoph Diehl [:posidron] 2012-05-09 18:56:35 PDT
Created attachment 622603 [details]
testcase
Comment 1 Christoph Diehl [:posidron] 2012-05-09 18:56:59 PDT
Created attachment 622604 [details]
callstack
Comment 2 martin_hosken 2012-05-09 20:10:54 PDT
I was unable to replicate this bug with valgrind and gr2fonttest (how I normally replicate such bugs). But a visual review of that area of the code has resulted in my tightening some checks. These have been pushed into the upstream repo. This bug will need to be retested when the patch with this repo version in is applied to your code. (repo version: 976 (d74a1988ba08))
Comment 3 Daniel Veditz [:dveditz] 2012-05-17 13:36:36 PDT
What are our plans to update the mozilla version of Graphite2 to the latest upstream?
Comment 4 Jonathan Kew (:jfkthame) 2012-05-22 07:05:05 PDT
We've just updated Graphite2 in bug 753230. Christoph, could you re-test this with a build that includes that patch, and confirm whether it's fixed? Thanks!
Comment 5 Christoph Diehl [:posidron] 2012-05-22 12:32:02 PDT
This bug here is still reproducible.
Comment 6 Jonathan Kew (:jfkthame) 2012-05-29 12:05:51 PDT
Is the call stack still identical to comment #1, or have line numbers etc possibly shifted at all in the Graphite update? If you could post a fresh call stack to confirm exactly where it's occurring, I'll try to take a look.
Comment 7 Christoph Diehl [:posidron] 2012-05-29 13:29:22 PDT
Created attachment 628091 [details]
fresh-callstack

Have a attach a fresh callstack.
Comment 8 martin_hosken 2012-05-29 23:19:11 PDT
Fix committed to upstream repository. Planning to do a release later today. BTW the latest valgrind has no exp-ptrcheck and exp-sgcheck doesn't trigger on this fault :(
Comment 9 Jonathan Kew (:jfkthame) 2012-05-30 03:11:36 PDT
Created attachment 628290 [details] [diff] [review]
patch from upstream

This is the relevant patch from the upstream repo - Christoph, if you have a chance to try this and confirm that it fixes the issue, that'd be great.

(Not requesting review on this for now as I plan to take the complete new release when it's ready in the next day or so, but in the meantime any testing would be welcome.)
Comment 10 Jonathan Kew (:jfkthame) 2012-05-30 04:01:40 PDT
Created attachment 628304 [details] [diff] [review]
update graphite2 to release 1.1.3

Version 1.1.3 has now been released upstream, so here's the patch to update our copy to the new version, which includes the fix for this bug.
Comment 11 Christoph Diehl [:posidron] 2012-05-30 04:18:29 PDT
Comment on attachment 628304 [details] [diff] [review]
update graphite2 to release 1.1.3

Review of attachment 628304 [details] [diff] [review]:
-----------------------------------------------------------------

Tested with the provieded patch, fixed!
Comment 12 John Daggett (:jtd) 2012-05-30 04:26:14 PDT
Comment on attachment 628304 [details] [diff] [review]
update graphite2 to release 1.1.3

If possible, having a crashtest for this would be nice.
Comment 13 Jonathan Kew (:jfkthame) 2012-05-30 06:27:22 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/1446e5aba066

Not sure about a crashtest - the issue only showed up under ASAN, not as a crash with a normal browser build (though in principle it could've crashed if the out-of-bounds access happened to hit an unmapped page, or something like that).

Maybe we should be running unit tests with ASAN builds, at least on a daily basis if not per-push? I expect that'd flush out some issues...
Comment 14 Ed Morley [:emorley] 2012-05-31 06:18:44 PDT
https://hg.mozilla.org/mozilla-central/rev/1446e5aba066
Comment 15 Daniel Veditz [:dveditz] 2012-06-21 17:16:43 PDT
We need these fixes in Beta (Fx14), right? Please request approval on the patch unless it's too risky to consider. (This may not be the only bug or security bug fixed upstream).

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