Closed Bug 1243843 Opened 8 years ago Closed 8 years ago

graphite2: crash around null in [@graphite2::Pass::collisionShift]

Categories

(Core :: Graphics: Text, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla47
Tracking Status
firefox44 --- wontfix
firefox45 + fixed
firefox46 + fixed
firefox47 + fixed

People

(Reporter: tsmith, Assigned: jfkthame)

References

(Blocks 1 open bug)

Details

(4 keywords, Whiteboard: [adv-main45-] Patch fixes multiple security bugs, not just this one)

Attachments

(3 files, 1 obsolete file)

Attached file test_case.ttf
This was found while fuzzing graphite2 1.3.5 and may not affect Firefox.

==4919==ERROR: AddressSanitizer: SEGV on unknown address 0x00000000002c (pc 0x7ffac1c2206c bp 0x7ffda3937130 sp 0x7ffda3937000 T0)
    #0 0x7ffac1c2206b in graphite2::SlotCollision::flags() const /home/user/Desktop/graphite2-1.3.5/src/inc/Collider.h:83:5
    #1 0x7ffac1c2206b in graphite2::Pass::collisionShift(graphite2::Segment*, int, graphite2::json*) const /home/user/Desktop/graphite2-1.3.5/src/Pass.cpp:742
    #2 0x7ffac1c1ead6 in graphite2::Pass::runGraphite(graphite2::vm::Machine&, graphite2::FiniteStateMachine&, bool) const /home/user/Desktop/graphite2-1.3.5/src/Pass.cpp:433:14
    #3 0x7ffac1c39c8a in graphite2::Silf::runGraphite(graphite2::Segment*, unsigned char, unsigned char, int) const /home/user/Desktop/graphite2-1.3.5/src/Silf.cpp:423:21
    #4 0x7ffac1bf8a78 in graphite2::Face::runGraphite(graphite2::Segment*, graphite2::Silf const*) const /home/user/Desktop/graphite2-1.3.5/src/Face.cpp:180:16
    #5 0x7ffac1bc7c6e in graphite2::Segment::runGraphite() /home/user/Desktop/graphite2-1.3.5/src/inc/Segment.h:97:45
    #6 0x7ffac1bc7c6e in (anonymous namespace)::makeAndInitialize(graphite2::Font const*, graphite2::Face const*, unsigned int, graphite2::FeatureVal const*, gr_encform, void const*, unsigned long, int) /home/user/Desktop/graphite2-1.3.5/src/gr_segment.cpp:46
    #7 0x7ffac1bc7c6e in gr_make_seg /home/user/Desktop/graphite2-1.3.5/src/gr_segment.cpp:105
    #8 0x4eccd9 in Gr2Renderer::renderText(char const*, unsigned long, RenderedLine*, _IO_FILE*) /home/user/Desktop/graphite2-1.3.5/tests/comparerenderer/Gr2Renderer.h:103:28
    #9 0x4edc73 in CompareRenderer::runRenderer(Renderer&, RenderedLine*, unsigned long&, _IO_FILE*) /home/user/Desktop/graphite2-1.3.5/tests/comparerenderer/CompareRenderer.cpp:213:17
    #10 0x4e6422 in CompareRenderer::runTests(_IO_FILE*, int) /home/user/Desktop/graphite2-1.3.5/tests/comparerenderer/CompareRenderer.cpp:126:41
    #11 0x4e6422 in main /home/user/Desktop/graphite2-1.3.5/tests/comparerenderer/CompareRenderer.cpp:385
    #12 0x7ffac1604ec4 in __libc_start_main /build/buildd/eglibc-2.19/csu/libc-start.c:287
    #13 0x41bc77 in _start (/home/user/Desktop/graphite/comparerenderer+0x41bc77)
Attached file test_string.txt
Pull in the latest upstream bugfixes. According to Martin, this should fix the recent clutch of graphite2 fuzzbugs. :tsmith, could you re-test against a build that includes this patch, before we actually land it?
Assignee: nobody → jfkthame
Status: NEW → ASSIGNED
Flags: needinfo?(twsmith)
Attachment #8714447 - Flags: review?(jd.bugzilla)
Attachment #8714447 - Flags: review?(jd.bugzilla) → review+
Fixed upstream in a8b3ac2aed0eb132cd80efe7de88f8153e73c829
Patch for esr38 only; this updates graphite2 to the latest upstream code, in effect a roll-up of all the graphite2 updates that we've taken on trunk over the past several months.
Comment on attachment 8717334 [details] [diff] [review]
Update graphite2 library to latest release on esr38

Argh, attached this to the wrong bug - obsoleting.
Attachment #8717334 - Attachment is obsolete: true
Comment on attachment 8714447 [details] [diff] [review]
Pull latest bugfixes from upstream graphite2 (now at e569e28d83491fedb31b9220493f3c07f6ec6d80)

Note that this patch fixes the whole clutch of recently-filed graphite2 bugs; it's not specific to just this one PoC.

[Security approval request comment]
How easily could an exploit be constructed based on the patch?
The fixes would make it fairly clear to someone familiar with the font format that specific parts are being "hardened" with added checks, and therefore where to look for flaws. It's not clear to me how readily this could be translated to an actual exploit.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
No

Which older supported branches are affected by this flaw?
All

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

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
Trivial to backport to aurora & beta. For esr38, see bug 1246093.

How likely is this patch to cause regressions; how much testing does it need?
Minimal risk of regressions; this is just fixing some missing error-/null-checking.
Attachment #8714447 - Flags: sec-approval?
Comment on attachment 8714447 [details] [diff] [review]
Pull latest bugfixes from upstream graphite2 (now at e569e28d83491fedb31b9220493f3c07f6ec6d80)

Approval Request Comment
[Feature/regressing bug #]: fixes from upstream for graphite2 fuzzbugs
[User impact if declined]: potential crash/DoS/exploit via malicious fonts
[Describe test coverage new/current, TreeHerder]: library fixes tested upstream; later, we could add the testcases from here and related bugs as crashtests.
[Risks and why]: low-risk fixes that add error checking
[String/UUID change made/needed]: n/a
Attachment #8714447 - Flags: approval-mozilla-beta?
Attachment #8714447 - Flags: approval-mozilla-aurora?
Comment on attachment 8714447 [details] [diff] [review]
Pull latest bugfixes from upstream graphite2 (now at e569e28d83491fedb31b9220493f3c07f6ec6d80)

Approvals given.

Release Management, this fixes a bunch of severe issues captured in other bugs.
Attachment #8714447 - Flags: sec-approval?
Attachment #8714447 - Flags: sec-approval+
Attachment #8714447 - Flags: approval-mozilla-beta?
Attachment #8714447 - Flags: approval-mozilla-beta+
Attachment #8714447 - Flags: approval-mozilla-aurora?
Attachment #8714447 - Flags: approval-mozilla-aurora+
[Tracking Requested - why for this release]:

Marking this as affecting all current releases and tracking since it's a security fix that we are uplifting.
In some ways it's unfortunate that this is the bug that the patch landed on. It's less compelling from a security POV than several of the other bugs we think it also fixes.
Whiteboard: Patch fixes multiple security bugs, not just this one
https://hg.mozilla.org/integration/mozilla-inbound/rev/bf9456e800490d363320df0803db717bb18a80fe
Bug 1243843 - Pull latest bugfixes from upstream graphite2 (now at e569e28d83491fedb31b9220493f3c07f6ec6d80). r=jdaggett
https://hg.mozilla.org/mozilla-central/rev/bf9456e80049
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
Tested with e569e28d83491fedb31b9220493f3c07f6ec6d80
Status: RESOLVED → VERIFIED
Flags: needinfo?(twsmith)
See Also: → CVE-2016-2791
Group: gfx-core-security → core-security-release
Whiteboard: Patch fixes multiple security bugs, not just this one → [adv-main45-] Patch fixes multiple security bugs, not just this one
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.