1252411 - Graphite2 heap-based overread in TtfUtil::CheckCmapSubtable12

Status: RESOLVED FIXED

Description

[Mateusz Jurczyk]

Created attachment 8725183 [details]
Reproducers.

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/48.0.2564.116 Safari/537.36

Steps to reproduce:

The following crash due to a heap-based buffer overread can be observed in an ASAN build of the standard Graphite2 gr2FontTest utility (git trunk), triggered with the following command:

$ ./gr2fonttest /path/to/file -auto

--- cut ---
=================================================================
==27862==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x61200000be45 at pc 0x0000005f3354 bp 0x7ffe1a7ac5b0 sp 0x7ffe1a7ac5a8
READ of size 4 at 0x61200000be45 thread T0
    #0 0x5f3353 in graphite2::TtfUtil::CheckCmapSubtable12(void const*, void const*) graphite/src/TtfUtil.cpp:1092:40
    #1 0x4fa415 in smp_subtable(graphite2::Face::Table const&) graphite/src/CmapCache.cpp:55:9
    #2 0x4fa859 in graphite2::CachedCmap::CachedCmap(graphite2::Face const&) graphite/src/CmapCache.cpp:95:29
    #3 0x54bf42 in graphite2::Face::readGlyphs(unsigned int) graphite/src/Face.cpp:108:22
    #4 0x56fb34 in (anonymous namespace)::load_face(graphite2::Face&, unsigned int) graphite/src/gr_face.cpp:54:14
    #5 0x56f644 in gr_make_face_with_ops graphite/src/gr_face.cpp:89:16
    #6 0x571980 in gr_make_file_face graphite/src/gr_face.cpp:242:23
    #7 0x4ecf13 in Parameters::testFileFont() const (graphite/gr2fonttest/gr2fonttest+0x4ecf13)
    #8 0x4f0387 in main (graphite/gr2fonttest/gr2fonttest+0x4f0387)

0x61200000be45 is located 1 bytes to the right of 260-byte region [0x61200000bd40,0x61200000be44)
allocated by thread T0 here:
    #0 0x4b85b8 in malloc llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:40
    #1 0x55dc0b in graphite2::FileFace::get_table_fn(void const*, unsigned int, unsigned long*) graphite/src/FileFace.cpp:94:11
    #2 0x54f8b1 in graphite2::Face::Table::Table(graphite2::Face const&, graphite2::TtfUtil::Tag, unsigned int) graphite/src/Face.cpp:280:36
    #3 0x4fa793 in graphite2::CachedCmap::CachedCmap(graphite2::Face const&) graphite/src/CmapCache.cpp:91:23
    #4 0x54bf42 in graphite2::Face::readGlyphs(unsigned int) graphite/src/Face.cpp:108:22
    #5 0x56fb34 in (anonymous namespace)::load_face(graphite2::Face&, unsigned int) graphite/src/gr_face.cpp:54:14
    #6 0x56f644 in gr_make_face_with_ops graphite/src/gr_face.cpp:89:16
    #7 0x571980 in gr_make_file_face graphite/src/gr_face.cpp:242:23
    #8 0x4ecf13 in Parameters::testFileFont() const (graphite/gr2fonttest/gr2fonttest+0x4ecf13)
    #9 0x4f0387 in main (graphite/gr2fonttest/gr2fonttest+0x4f0387)

SUMMARY: AddressSanitizer: heap-buffer-overflow graphite/src/TtfUtil.cpp:1092:40 in graphite2::TtfUtil::CheckCmapSubtable12(void const*, void const*)
Shadow bytes around the buggy address:
  0x0c247fff9770: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c247fff9780: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c247fff9790: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c247fff97a0: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
  0x0c247fff97b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c247fff97c0: 00 00 00 00 00 00 00 00[04]fa fa fa fa fa fa fa
  0x0c247fff97d0: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
  0x0c247fff97e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c247fff97f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 fa fa
  0x0c247fff9800: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c247fff9810: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Heap right redzone:      fb
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack partial redzone:   f4
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==27862==ABORTING
--- cut ---

Attached is an archive with three font files which reproduce the crash.

This bug is subject to a 90 day disclosure deadline. If 90 days elapse without a broadly available patch, then the bug report will automatically become visible to the public.

Comment 2

[martin_hosken]

fixed? in f191776eaf5f90de29e6bf6771dee070d1349857

Comment 3

[Tyson Smith [:tsmith]]

Verified with graphite revision 520d76818052772d614e581dacea69499b912be6.

Mateusz can you please confirm? This will be marked fixed once the patches land in our branches.

Comment 4

[Mateusz Jurczyk]

Created attachment 8727394 [details]
NULL pointer dereference reproducer.

I confirm that the crashes reported in this bug no longer reproduce with the latest revision.

However, one of the corresponding testcases still triggers an unrelated NULL pointer dereference in src/GlyphCache.cpp:156, as a result of a failed allocation in line 151. I attached the input in question.

Comment 5

[Tyson Smith [:tsmith]]

(In reply to Mateusz Jurczyk from comment #4)
> However, one of the corresponding testcases still triggers an unrelated NULL
> pointer dereference in src/GlyphCache.cpp:156, as a result of a failed
> allocation in line 151. I attached the input in question.

NULL deref logged as bug 1254196.

Comment 6

[Daniel Veditz [:dveditz]]

Graphite2 has been updated on all affected branches including ESRs.