1356607 - (CVE-2017-7776) Graphite2: heap-buffer-overflow read [@ graphite2::Silf::getClassGlyph]

Status: RESOLVED FIXED

(Core :: Graphics: Text, defect)

Description

[Tyson Smith [:tsmith]]

Attachment: 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

[martin_hosken]

fixed in e04a302.

Comment 2

[Tyson Smith [:tsmith]]

(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

[martin_hosken]

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?

Comment 4

[Tyson Smith [:tsmith]]

(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.

Comment 5

[Tyson Smith [:tsmith]]

(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

[martin_hosken]

Must be. Sorry. Try 0646e4e.

Comment 7

[martin_hosken]

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?

Comment 8

[Jonathan Kew [:jfkthame]]

Fine with me, if Tyson & co are happy with it. (I assume you've arranged this in consultation with them.)

Comment 9

[Jonathan Kew [:jfkthame]]

(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.

Comment 10

[Tyson Smith [:tsmith]]

Verified fixed in graphite-security commit bb24c61

Comment 11

[Jonathan Kew [:jfkthame]]

Should be fixed by the patch landed in bug 1349310.

Comment 12

[Frederik Braun [:freddy]]

(In reply to Tyson Smith [:tsmith] from comment #10)
> Verified fixed in graphite-security commit bb24c61