Graphite2 two heap-based overreads in GlyphCache::Loader

RESOLVED FIXED

Status

()

Product:
Core
Component:
Graphics: Text
RESOLVED FIXED
Reported:
2 years ago
Modified:
a year ago

People

(Reporter: Mateusz Jurczyk, Assigned: martin_hosken)

Tracking

(4 keywords)

Version:
unspecified
Keywords:
crash, csectype-bounds, sec-moderate, testcase
Points:
---
Duplicates:
1253239

Firefox Tracking Flags

(firefox45 disabled, firefox46 fixed, firefox47 fixed, firefox48 fixed, firefox-esr3846+ disabled, firefox-esr4546+ disabled)

Details

(Whiteboard: Disclosure deadline May 30 [gfx-noted])

Attachments

(2 attachments)

Reproducers.
29.57 KB, application/zip
Details
NULL pointer dereference reproducer.
2.48 KB, application/x-font-otf
Details
(Reporter)

Description

•
2 years ago 
Created attachment 8725177 [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 crashes due to two different heap-based buffer overreads 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

While we have seen the crashes to occur with six unique call stacks, eventually the OOB reads happen at two code locations: graphite2::GlyphCache::Loader::Loader (graphite/src/GlyphCache.cpp:306:38) and graphite2::GlyphCache::Loader::read_glyph (graphite/src/GlyphCache.cpp:398:27). Below you can see the ASAN reports of crashes in both functions:

--- cut ---
=================================================================
==26347==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60c00000bf40 at pc 0x00000055445d bp 0x7ffe231e8130 sp 0x7ffe231e8128
READ of size 1 at 0x60c00000bf40 thread T0
    #0 0x55445c in unsigned long be::_peek<1>(unsigned char const*) graphite/src/./inc/Endian.h:77:73
    #1 0x5543c8 in unsigned long be::_peek<2>(unsigned char const*) graphite/src/./inc/Endian.h:50:43
    #2 0x551eab in unsigned short be::read<unsigned short>(unsigned char const*&) graphite/src/./inc/Endian.h:60:23
    #3 0x562a66 in graphite2::GlyphCache::Loader::read_glyph(unsigned short, graphite2::GlyphFace&, int*) const graphite/src/GlyphCache.cpp:398:27
    #4 0x560481 in graphite2::GlyphCache::GlyphCache(graphite2::Face const&, unsigned int) graphite/src/GlyphCache.cpp:142:37
    #5 0x54bb13 in graphite2::Face::readGlyphs(unsigned int) graphite/src/Face.cpp:98:29
    #6 0x56fb34 in (anonymous namespace)::load_face(graphite2::Face&, unsigned int) graphite/src/gr_face.cpp:54:14
    #7 0x56f644 in gr_make_face_with_ops graphite/src/gr_face.cpp:89:16
    #8 0x571980 in gr_make_file_face graphite/src/gr_face.cpp:242:23
    #9 0x4ecf13 in Parameters::testFileFont() const (graphite/gr2fonttest/gr2fonttest+0x4ecf13)
    #10 0x4f0387 in main (graphite/gr2fonttest/gr2fonttest+0x4f0387)

0x60c00000bf40 is located 0 bytes to the right of 128-byte region [0x60c00000bec0,0x60c00000bf40)
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 0x567867 in graphite2::GlyphCache::Loader::Loader(graphite2::Face const&, bool) graphite/src/GlyphCache.cpp:268:24
    #4 0x55ef50 in graphite2::GlyphCache::GlyphCache(graphite2::Face const&, unsigned int) graphite/src/GlyphCache.cpp:118:21
    #5 0x54bb13 in graphite2::Face::readGlyphs(unsigned int) graphite/src/Face.cpp:98:29
    #6 0x56fb34 in (anonymous namespace)::load_face(graphite2::Face&, unsigned int) graphite/src/gr_face.cpp:54:14
    #7 0x56f644 in gr_make_face_with_ops graphite/src/gr_face.cpp:89:16
    #8 0x571980 in gr_make_file_face graphite/src/gr_face.cpp:242:23
    #9 0x4ecf13 in Parameters::testFileFont() const (graphite/gr2fonttest/gr2fonttest+0x4ecf13)
    #10 0x4f0387 in main (graphite/gr2fonttest/gr2fonttest+0x4f0387)

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:
  0x0c187fff9790: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c187fff97a0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c187fff97b0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c187fff97c0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c187fff97d0: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
=>0x0c187fff97e0: 00 00 00 00 00 00 00 00[fa]fa fa fa fa fa fa fa
  0x0c187fff97f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 07
  0x0c187fff9800: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c187fff9810: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c187fff9820: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c187fff9830: 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
==26347==ABORTING
--- cut ---

--- cut ---
=================================================================
==26561==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60200000efb7 at pc 0x00000055445d bp 0x7ffc518d4260 sp 0x7ffc518d4258
READ of size 1 at 0x60200000efb7 thread T0
    #0 0x55445c in unsigned long be::_peek<1>(unsigned char const*) graphite/src/./inc/Endian.h:77:73
    #1 0x5543c8 in unsigned long be::_peek<2>(unsigned char const*) graphite/src/./inc/Endian.h:50:43
    #2 0x554358 in unsigned long be::_peek<4>(unsigned char const*) graphite/src/./inc/Endian.h:50:43
    #3 0x551d6b in unsigned int be::read<unsigned int>(unsigned char const*&) graphite/src/./inc/Endian.h:60:23
    #4 0x5685a5 in graphite2::GlyphCache::Loader::Loader(graphite2::Face const&, bool) graphite/src/GlyphCache.cpp:306:38
    #5 0x55ef50 in graphite2::GlyphCache::GlyphCache(graphite2::Face const&, unsigned int) graphite/src/GlyphCache.cpp:118:21
    #6 0x54bb13 in graphite2::Face::readGlyphs(unsigned int) graphite/src/Face.cpp:98:29
    #7 0x56fb34 in (anonymous namespace)::load_face(graphite2::Face&, unsigned int) graphite/src/gr_face.cpp:54:14
    #8 0x56f644 in gr_make_face_with_ops graphite/src/gr_face.cpp:89:16
    #9 0x571980 in gr_make_file_face graphite/src/gr_face.cpp:242:23
    #10 0x4ecf13 in Parameters::testFileFont() const (graphite/gr2fonttest/gr2fonttest+0x4ecf13)
    #11 0x4f0387 in main (graphite/gr2fonttest/gr2fonttest+0x4f0387)

0x60200000efb7 is located 0 bytes to the right of 7-byte region [0x60200000efb0,0x60200000efb7)
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 0x567867 in graphite2::GlyphCache::Loader::Loader(graphite2::Face const&, bool) graphite/src/GlyphCache.cpp:268:24
    #4 0x55ef50 in graphite2::GlyphCache::GlyphCache(graphite2::Face const&, unsigned int) graphite/src/GlyphCache.cpp:118:21
    #5 0x54bb13 in graphite2::Face::readGlyphs(unsigned int) graphite/src/Face.cpp:98:29
    #6 0x56fb34 in (anonymous namespace)::load_face(graphite2::Face&, unsigned int) graphite/src/gr_face.cpp:54:14
    #7 0x56f644 in gr_make_face_with_ops graphite/src/gr_face.cpp:89:16
    #8 0x571980 in gr_make_file_face graphite/src/gr_face.cpp:242:23
    #9 0x4ecf13 in Parameters::testFileFont() const (graphite/gr2fonttest/gr2fonttest+0x4ecf13)
    #10 0x4f0387 in main (graphite/gr2fonttest/gr2fonttest+0x4f0387)

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:
  0x0c047fff9da0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9db0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9dc0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9dd0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9de0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x0c047fff9df0: fa fa fa fa fa fa[07]fa fa fa 06 fa fa fa 00 04
  0x0c047fff9e00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9e10: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9e20: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9e30: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9e40: 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
==26561==ABORTING
--- cut ---

Attached is an archive with three font files per each unique crash (in terms of stack trace). There are two directories with reproducers for the graphite2::GlyphCache::Loader::read_glyph crash and four directories with reproducers for graphite2::GlyphCache::Loader::Loader.

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.
Group: core-security → gfx-core-security
Status: UNCONFIRMED → NEW
Ever confirmed: true
Whiteboard: Disclosure deadline May 30

Updated

•
2 years ago
Keywords: crash, csectype-bounds, testcase

Updated

•
2 years ago
Duplicate of this bug: 1253239
 (Assignee)

Comment 2

•
2 years ago 
fixed? before or in e7deaf90c9c8ca30116340419313af527fe90d78

Comment 3

•
2 years ago 
Verified with graphite revision 520d76818052772d614e581dacea69499b912be6.

Mateusz can you please confirm? This will be marked fixed once the patches land in our branches.
Flags: needinfo?(j00ru.vx)
Whiteboard: Disclosure deadline May 30 → Disclosure deadline May 30 [gfx-noted]
 (Reporter)

Comment 4

•
2 years ago 
Created attachment 8727393 [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.
Flags: needinfo?(j00ru.vx)

Updated

•
2 years ago
See Also: → bug 1254196

Comment 5

•
2 years ago 
(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.

I have created a separate bug for the NULL deref. bug 1254196. That test case doesn't seem to be in the original zip file.
 (Reporter)

Comment 6

•
2 years ago 
Yes, while the fuzzer generated multiple test cases for the original OOB reads reported here, I only submitted three per stack trace here. This new one wasn't one of them I guess.
Keywords: sec-moderate

Updated

•
2 years ago
Depends on: 1255158
Assignee: nobody → martin_hosken

Updated

•
2 years ago
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Group: gfx-core-security → core-security-release

Comment 7

•
2 years ago 
Graphite2 has been updated on all affected branches including ESRs.
status-firefox45: --- → wontfix
status-firefox46: --- → fixed
status-firefox47: --- → fixed
status-firefox48: --- → fixed
status-firefox-esr38: --- → fixed
status-firefox-esr45: --- → fixed
tracking-firefox-esr38: --- → 46+
tracking-firefox-esr45: --- → 46+
status-firefox45: wontfix → disabled
status-firefox-esr38: fixed → disabled
status-firefox-esr45: fixed → disabled
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.