Bug 1356607 (CVE-2017-7776)

Graphite2: heap-buffer-overflow read [@ graphite2::Silf::getClassGlyph]

RESOLVED FIXED in Firefox -esr52

Status

()

defect
--
critical
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: tsmith, Assigned: jfkthame)

Tracking

(Blocks 1 bug, 4 keywords)

Trunk
mozilla55
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox-esr45 wontfix, firefox-esr5254+ fixed, firefox53 wontfix, firefox54 fixed, firefox55 fixed)

Details

(Whiteboard: [post-critsmash-triage][adv-main54+][adv-esr52.2+])

Attachments

(1 attachment)

45.50 KB, application/x-font-ttf
Details
Reporter

Description

2 years ago
Posted file test_case.ttf
Found in graphite git revision 8afc7d00.

STR:
With an ASan build run:
./g2fonttest test_case.ttf -auto -demand

==21704==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60d0000000c6 at pc 0x0000005702c7 bp 0x7ffefba26fd0 sp 0x7ffefba26fc8
READ of size 2 at 0x60d0000000c6 thread T0
    #0 0x5702c6 in graphite2::Silf::getClassGlyph(unsigned short, unsigned int) const src/Silf.cpp:349:17
    #1 0x5f2253 in graphite2::Segment::getClassGlyph(unsigned short, unsigned short) const src/inc/Segment.h:128:76
    #2 0x5f2253 in (anonymous namespace)::put_glyph_8bit_obs(unsigned char const*&, int*&, int*, regbank&) src/inc/opcodes.h:226
    #3 0x5ee2f4 in graphite2::vm::Machine::run(void* const*, unsigned char const*, graphite2::Slot**&) src/call_machine.cpp:121:12
    #4 0x5eb4a9 in graphite2::vm::Machine::Code::run(graphite2::vm::Machine&, graphite2::Slot**&) const src/Code.cpp:746:15
    #5 0x5d567f in graphite2::Pass::doAction(graphite2::vm::Machine::Code const*, graphite2::Slot*&, graphite2::vm::Machine&) const src/Pass.cpp:682:26
    #6 0x5cf59b in graphite2::Pass::findNDoRule(graphite2::Slot*&, graphite2::vm::Machine&, graphite2::FiniteStateMachine&) const src/Pass.cpp:548:33
    #7 0x5cda88 in graphite2::Pass::runGraphite(graphite2::vm::Machine&, graphite2::FiniteStateMachine&, bool) const src/Pass.cpp:417:13
    #8 0x571411 in graphite2::Silf::runGraphite(graphite2::Segment*, unsigned char, unsigned char, int) const src/Silf.cpp:429:33
    #9 0x533dea in graphite2::Face::runGraphite(graphite2::Segment*, graphite2::Silf const*) const src/Face.cpp:180:23
    #10 0x527695 in graphite2::Segment::runGraphite() src/inc/Segment.h:97:53
    #11 0x524b62 in (anonymous namespace)::makeAndInitialize(graphite2::Font const*, graphite2::Face const*, unsigned int, graphite2::FeatureVal const*, gr_encform, void const*, unsigned long, int) src/gr_segment.cpp:46:73
    #12 0x524b62 in gr_make_seg src/gr_segment.cpp:105
    #13 0x51aaea in Parameters::testFileFont() const gr2fonttest/gr2FontTest.cpp
    #14 0x51c809 in main gr2fonttest/gr2FontTest.cpp:798:20
    #15 0x7f778a1a482f in __libc_start_main /build/glibc-9tT8Do/glibc-2.23/csu/../csu/libc-start.c:291
    #16 0x41d498 in _start (gr2fonttest+0x41d498)

Comment 1

2 years ago
fixed in e04a302.
Reporter

Comment 2

2 years ago
(In reply to martin_hosken from comment #1)
> fixed in e04a302.

Martin, Please coordinate releases with us. When security bugs are patched in the open without allowing us to patch and release  in sync with Graphite Firefox users are left vulnerable for sometimes weeks at a time.

Comment 3

2 years ago
Do you have a secure server where we could keep a graphite git repo and push to that for these kinds of bugs for you to check and then on release I can merge that into the public repo?
Reporter

Comment 4

2 years ago
(In reply to martin_hosken from comment #3)
> Do you have a secure server where we could keep a graphite git repo and push
> to that for these kinds of bugs for you to check and then on release I can
> merge that into the public repo?

I will contact you via email to discuss.
Reporter

Comment 5

2 years ago
(In reply to martin_hosken from comment #1)
> fixed in e04a302.

This does seem to be fixed but git commit e04a302 does not exist. Typo perhaps?

Comment 6

2 years ago
Must be. Sorry. Try 0646e4e.

Comment 7

2 years ago
OK There is now a new private repo https://github.com/sillsdev/graphite-security that has a mozilla-security branch into which I can place bug fixes for security bugs for you guys to test. Let me know your github id so I can add you as a collaborator. Notice that this branch will only get security fixes for testing, this repo is not the primary repo and any code pulls into the mozilla-central tree need to still come from the main graphite repo. We can pull stuff across from graphite-security to graphite when needed, just let me know.

JK are you happy with this new approach?
Flags: needinfo?(jfkthame)
Assignee

Comment 8

2 years ago
Fine with me, if Tyson & co are happy with it. (I assume you've arranged this in consultation with them.)
Flags: needinfo?(jfkthame)

Updated

2 years ago
Flags: needinfo?(twsmith)
Assignee

Comment 9

2 years ago
(In reply to martin_hosken from comment #7)
> OK There is now a new private repo
> https://github.com/sillsdev/graphite-security that has a mozilla-security
> branch into which I can place bug fixes for security bugs for you guys to
> test. Let me know your github id so I can add you as a collaborator.

Oh, btw - I'm jfkthame on github.
Reporter

Comment 10

2 years ago
Verified fixed in graphite-security commit bb24c61
Flags: needinfo?(twsmith)
Keywords: sec-high
Assignee

Comment 11

2 years ago
Should be fixed by the patch landed in bug 1349310.
Depends on: CVE-2017-7778
(In reply to Tyson Smith [:tsmith] from comment #10)
> Verified fixed in graphite-security commit bb24c61
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Assignee: nobody → jfkthame
Target Milestone: --- → mozilla55
Group: gfx-core-security → core-security-release
Flags: qe-verify-
Whiteboard: [post-critsmash-triage]
Whiteboard: [post-critsmash-triage] → [post-critsmash-triage][adv-main54+][adv-esr52.2+]
Alias: CVE-2017-7776
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.