1254497 - Graphite2 multiple heap-based out-of-bounds reads in NameTable::getName

Status: RESOLVED FIXED

(Core :: Graphics: Text, defect)

Description

[Mateusz Jurczyk]

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

We have encountered several different crashes in the graphite2::NameTable::getName method, observed in an ASAN build of the standard Graphite2 gr2FontTest utility (git trunk), triggered with the following command:

$ ./gr2fonttest -demand -cache /path/to/file

Below are three unique ASAN reports that we have triggered.

--- cut ---
==1191==ERROR: AddressSanitizer: SEGV on unknown address 0x61b000026b15 (pc 0x000000553c81 bp 0x7ffc0e24a820 sp 0x7ffc0e24a800 T0)
    #0 0x553c80 in unsigned long be::_peek<1>(unsigned char const*) graphite/src/./inc/Endian.h:77:73
    #1 0x553bd3 in unsigned long be::_peek<2>(unsigned char const*) graphite/src/./inc/Endian.h:50:16
    #2 0x5516cb in unsigned short be::read<unsigned short>(unsigned char const*&) graphite/src/./inc/Endian.h:60:23
    #3 0x59192b in graphite2::NameTable::getName(unsigned short&, unsigned short, gr_encform, unsigned int&) graphite/src/NameTable.cpp:157:24
    #4 0x572e5c in gr_fref_label graphite/src/gr_features.cpp:97:12
    #5 0x4eaec8 in Parameters::printFeatures(gr_face const*) const (graphite/gr2fonttest/gr2fonttest+0x4eaec8)
    #6 0x4ed32b in Parameters::testFileFont() const (graphite/gr2fonttest/gr2fonttest+0x4ed32b)
    #7 0x4f06c9 in main (graphite/gr2fonttest/gr2fonttest+0x4f06c9)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV graphite/src/./inc/Endian.h:77:73 in unsigned long be::_peek<1>(unsigned char const*)
==1191==ABORTING
--- cut ---

--- cut ---
==1199==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x61b00001fb95 at pc 0x000000553c7d bp 0x7ffdebef2a70 sp 0x7ffdebef2a68
READ of size 1 at 0x61b00001fb95 thread T0
    #0 0x553c7c in unsigned long be::_peek<1>(unsigned char const*) graphite/src/./inc/Endian.h:77:73
    #1 0x553bd3 in unsigned long be::_peek<2>(unsigned char const*) graphite/src/./inc/Endian.h:50:16
    #2 0x5516cb in unsigned short be::read<unsigned short>(unsigned char const*&) graphite/src/./inc/Endian.h:60:23
    #3 0x59192b in graphite2::NameTable::getName(unsigned short&, unsigned short, gr_encform, unsigned int&) graphite/src/NameTable.cpp:157:24
    #4 0x572e5c in gr_fref_label graphite/src/gr_features.cpp:97:12
    #5 0x4eaec8 in Parameters::printFeatures(gr_face const*) const (graphite/gr2fonttest/gr2fonttest+0x4eaec8)
    #6 0x4ed32b in Parameters::testFileFont() const (graphite/gr2fonttest/gr2fonttest+0x4ed32b)
    #7 0x4f06c9 in main (graphite/gr2fonttest/gr2fonttest+0x4f06c9)

AddressSanitizer can not describe address in more detail (wild memory access suspected).
SUMMARY: AddressSanitizer: heap-buffer-overflow graphite/src/./inc/Endian.h:77:73 in unsigned long be::_peek<1>(unsigned char const*)
Shadow bytes around the buggy address:
  0x0c367fffbf20: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c367fffbf30: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c367fffbf40: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c367fffbf50: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c367fffbf60: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x0c367fffbf70: fa fa[fa]fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c367fffbf80: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c367fffbf90: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c367fffbfa0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c367fffbfb0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c367fffbfc0: 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
==1199==ABORTING
--- cut ---

--- cut ---
==1315==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60400000db3a at pc 0x00000057d59d bp 0x7ffd01d33840 sp 0x7ffd01d33838
READ of size 2 at 0x60400000db3a thread T0
    #0 0x57d59c in graphite2::_utf_codec<16>::get(unsigned short const*, signed char&) graphite/src/./inc/UtfCodec.h:97:27
    #1 0x57d0a7 in graphite2::_utf_iterator<unsigned short const>::reference::operator unsigned int() const graphite/src/./inc/UtfCodec.h:173:74
    #2 0x591d32 in graphite2::NameTable::getName(unsigned short&, unsigned short, gr_encform, unsigned int&) graphite/src/NameTable.cpp:173:18
    #3 0x572e5c in gr_fref_label graphite/src/gr_features.cpp:97:12
    #4 0x4eaec8 in Parameters::printFeatures(gr_face const*) const (graphite/gr2fonttest/gr2fonttest+0x4eaec8)
    #5 0x4ed32b in Parameters::testFileFont() const (graphite/gr2fonttest/gr2fonttest+0x4ed32b)
    #6 0x4f06c9 in main (graphite/gr2fonttest/gr2fonttest+0x4f06c9)

0x60400000db3a is located 0 bytes to the right of 42-byte region [0x60400000db10,0x60400000db3a)
allocated by thread T0 here:
    #0 0x4b85b8 in malloc llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:40
    #1 0x55a24a in unsigned short* graphite2::gralloc<unsigned short>(unsigned long) graphite/src/./inc/Main.h:88:28
    #2 0x5916ef in graphite2::NameTable::getName(unsigned short&, unsigned short, gr_encform, unsigned int&) graphite/src/NameTable.cpp:147:37
    #3 0x572e5c in gr_fref_label graphite/src/gr_features.cpp:97:12
    #4 0x4eaec8 in Parameters::printFeatures(gr_face const*) const (graphite/gr2fonttest/gr2fonttest+0x4eaec8)
    #5 0x4ed32b in Parameters::testFileFont() const (graphite/gr2fonttest/gr2fonttest+0x4ed32b)
    #6 0x4f06c9 in main (graphite/gr2fonttest/gr2fonttest+0x4f06c9)

SUMMARY: AddressSanitizer: heap-buffer-overflow graphite/src/./inc/UtfCodec.h:97:27 in graphite2::_utf_codec<16>::get(unsigned short const*, signed char&)
Shadow bytes around the buggy address:
  0x0c087fff9b10: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c087fff9b20: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c087fff9b30: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c087fff9b40: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c087fff9b50: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x0c087fff9b60: fa fa 00 00 00 00 00[02]fa fa fd fd fd fd fd fd
  0x0c087fff9b70: fa fa fd fd fd fd fd fa fa fa fd fd fd fd fd fd
  0x0c087fff9b80: fa fa fd fd fd fd fd fd fa fa 00 00 00 00 00 00
  0x0c087fff9b90: fa fa 00 00 00 00 00 fa fa fa fd fd fd fd fd fa
  0x0c087fff9ba0: fa fa fd fd fd fd fd fd fa fa fd fd fd fd fd fa
  0x0c087fff9bb0: fa fa fd fd fd fd fd fa fa fa fd fd fd fd fd fd
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
==1315==ABORTING
--- cut ---

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

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 1

[martin_hosken]

Please bear in mind that all buggy fonts that trigger bugs related to font tables other than graphite tables may well not pass OTS and so may not be exploitable within firefox.

Comment 2

[martin_hosken]

This is a bug in the segcache that is not compiled into firefox. There is no value to Mozilla fuzzing -cache. Suggest lowering security rating. Fixed? in f67e446f6637d5845a4df55e83a4f8a0eb7ad42b

Comment 3

[martin_hosken]

Hmm. This is odd, the bug I see exposed is in the cache because I actually ran some data. OK. Fixed? in 67a4b3d992cc08f531bbde7987208be6e67bace8. This bug could get exercised since OTS is unlikely to be testing for bad UTF16 sequences in name strings. But that is dependent on firefox accessing feature name strings, which I'm not sure if it does.

Comment 4

[Mateusz Jurczyk]

FWIW, all of the crashes reported in this bug also reproduced without the -cache or -demand options. I only mentioned them because they were the flags I originally used for fuzzing.

Comment 5

[martin_hosken]

Yes. Once I tracked down what the real issue was, it has nothing to do with caching and all to do with whether firefox uses feature string names. I can't find evidence it does.

Comment 6

[Tyson Smith [:tsmith]]

(In reply to martin_hosken from comment #5)
> Yes. Once I tracked down what the real issue was, it has nothing to do with
> caching and all to do with whether firefox uses feature string names. I
> can't find evidence it does.

Jonathan can you confirm this?

Comment 7

[Jonathan Kew [:jfkthame]]

(In reply to Tyson Smith [:tsmith] from comment #6)
> (In reply to martin_hosken from comment #5)
> > Yes. Once I tracked down what the real issue was, it has nothing to do with
> > caching and all to do with whether firefox uses feature string names. I
> > can't find evidence it does.
> 
> Jonathan can you confirm this?

It's correct. Firefox accessed graphite features by ID (tag), but does not look at their string names.

Comment 8

[Tyson Smith [:tsmith]]

Lowering security rating because Firefox is unaffected. See comment #7. This could be bad for other applications that use graphite.

Comment 9

[Tyson Smith [:tsmith]]

Verified with graphite revision c9f2e111409ca160d3efc46daca83bb9fafab289

Comment 10

[Daniel Veditz [:dveditz]]

Graphite2 has been updated to 1.3.8 on all the relevant branches including ESRs