1439071 - Graphite2: heap-buffer-overflow in [@ graphite2::Pass::readPass]

Status: RESOLVED FIXED

(Core :: Graphics: Text, defect)

Description

[Tyson Smith [:tsmith]]

Attachment: testcase.ttf

Found in graphite commit 99658129785a218556929db0595a002a668b40b0

This bug seem to have been introduced by commit 7747bc1f4afb2b606332699a150b015b30d2817c

$ ./gr2fonttest testcase.ttf -auto
==28018==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x61600000fbed at pc 0x0000005602fb bp 0x7ffd2b4e2af0 sp 0x7ffd2b4e2ae8
READ of size 1 at 0x61600000fbed thread T0
    #0 0x5602fa in unsigned long be::_peek<1>(unsigned char const*) /graphite/src/inc/Endian.h:77:73
    #1 0x5602fa in unsigned long be::_peek<2>(unsigned char const*) /graphite/src/inc/Endian.h:50
    #2 0x5602fa in unsigned short be::peek<unsigned short>(void const*) /graphite/src/inc/Endian.h:55
    #3 0x5602fa in graphite2::Pass::readPass(unsigned char const*, unsigned long, unsigned long, graphite2::Face&, graphite2::passtype, unsigned int, graphite2::Error&) /graphite/src/Pass.cpp:183
    #4 0x526ef8 in graphite2::Silf::readGraphite(unsigned char const*, unsigned long, graphite2::Face&, unsigned int) /graphite/src/Silf.cpp:216:14
    #5 0x500824 in graphite2::Face::readGraphite(graphite2::Face::Table const&) /graphite/src/Face.cpp:149:14
    #6 0x4f773d in (anonymous namespace)::load_face(graphite2::Face&, unsigned int) /graphite/src/gr_face.cpp:59:42
    #7 0x4f773d in gr_make_face_with_ops /graphite/src/gr_face.cpp:89
    #8 0x4f8cba in gr_make_file_face /graphite/src/gr_face.cpp:242:23
    #9 0x4f223d in Parameters::testFileFont() const /graphite/gr2fonttest/gr2FontTest.cpp:638:20
    #10 0x4f410d in main /graphite/gr2fonttest/gr2FontTest.cpp:795:12
    #11 0x7f388eb0082f in __libc_start_main /build/glibc-Cl5G7W/glibc-2.23/csu/../csu/libc-start.c:291
    #12 0x419768 in _start (/graphite/build/gr2fonttest/gr2fonttest+0x419768)

0x61600000fbed is located 2 bytes to the right of 619-byte region [0x61600000f980,0x61600000fbeb)
allocated by thread T0 here:
    #0 0x4b9898 in __interceptor_malloc (/graphite/build/gr2fonttest/gr2fonttest+0x4b9898)
    #1 0x53ac12 in graphite2::FileFace::get_table_fn(void const*, unsigned int, unsigned long*) /graphite/src/FileFace.cpp:94:11
    #2 0x5016e8 in graphite2::Face::Table::Table(graphite2::Face const&, graphite2::TtfUtil::Tag, unsigned int) /graphite/src/Face.cpp:282:36
    #3 0x4f75ec in (anonymous namespace)::load_face(graphite2::Face&, unsigned int) /graphite/src/gr_face.cpp:49:21
    #4 0x4f75ec in gr_make_face_with_ops /graphite/src/gr_face.cpp:89
    #5 0x4f8cba in gr_make_file_face /graphite/src/gr_face.cpp:242:23

Comment 1

[martin_hosken]

Fixed? in e7c32b07480d1e1ab4325b0754bd3f8d77f0817c. Yes when fixing the wraparound calculation, the implicit check on p >= pass_end was dropped.

Comment 2

[Tyson Smith [:tsmith]]

verified fixed with commit e7c32b07480d1e1ab4325b0754bd3f8d77f0817c in the graphite-security repo

Comment 3

[Ryan VanderMeulen [:RyanVM]]

Firefox is unaffected by this issue since we're using version 1.3.10 and the commit responsible for this issue was landed upstream after that release.