Closed Bug 1098497 (CVE-2015-0823) Opened 10 years ago Closed 10 years ago

Heap-use-after-free in ots::ots_gasp_parse

Categories

(Core :: Graphics: Text, defect)

defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla36
Tracking Status
firefox36 --- fixed
firefox-esr31 --- wontfix
b2g-v2.2 --- fixed

People

(Reporter: attekett, Assigned: jfkthame)

Details

(Keywords: csectype-uaf, sec-low, Whiteboard: [adv-main36+])

Attachments

(3 files)

Attached file repro-file.html
Tested on:

OS: Ubuntu 14.04

Firefox: ASAN build from https://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-linux64-asan/1415892131/

ASAN-trace:

==7261==ERROR: AddressSanitizer: heap-use-after-free on address 0x603000128cb0 at pc 0x7f369587823a bp 0x7fff0897db50 sp 0x7fff0897db48
READ of size 2 at 0x603000128cb0 thread T0 (Web Content)
    #0 0x7f3695878239 in ots::ots_gasp_parse(ots::OpenTypeFile*, unsigned char const*, unsigned long) /builds/slave/m-cen-l64-asan-000000000000000/build/gfx/ots/src/gasp.cc:36:0
    #1 0x7f36958d8955 in (anonymous namespace)::ProcessGeneric(ots::OpenTypeFile*, unsigned int, ots::OTSStream*, unsigned char const*, unsigned long, std::vector<(anonymous namespace)::OpenTypeTable, std::allocator<(anonymous namespace)::OpenTypeTable> > const&, ots::Buffer&) /builds/slave/m-cen-l64-asan-000000000000000/build/gfx/ots/src/ots.cc:590:0
    #2 0x7f36958d0411 in (anonymous namespace)::ProcessTTF(ots::OpenTypeFile*, ots::OTSStream*, unsigned char const*, unsigned long) /builds/slave/m-cen-l64-asan-000000000000000/build/gfx/ots/src/ots.cc:237:0
    #3 0x7f36958cd7a5 in ots::OTSContext::Process(ots::OTSStream*, unsigned char const*, unsigned long) /builds/slave/m-cen-l64-asan-000000000000000/build/gfx/ots/src/ots.cc:824:0
    #4 0x7f36902d64f3 in gfxUserFontEntry::SanitizeOpenTypeData(unsigned char const*, unsigned int, unsigned int&, gfxUserFontType) /builds/slave/m-cen-l64-asan-000000000000000/build/gfx/thebes/gfxUserFontSet.cpp:253:0
    #5 0x7f36902daa19 in gfxUserFontEntry::LoadPlatformFont(unsigned char const*, unsigned int&) /builds/slave/m-cen-l64-asan-000000000000000/build/gfx/thebes/gfxUserFontSet.cpp:600:0
    #6 0x7f36902d79d4 in gfxUserFontEntry::LoadNextSrc() /builds/slave/m-cen-l64-asan-000000000000000/build/gfx/thebes/gfxUserFontSet.cpp:488:0
    #7 0x7f36902de94f in Load /builds/slave/m-cen-l64-asan-000000000000000/build/gfx/thebes/gfxUserFontSet.cpp:686:0
    #8 0x7f36902de94f in gfxUserFontSet::FindUserFontEntryAndLoad(gfxFontFamily*, gfxFontStyle const&, bool&, bool&) /builds/slave/m-cen-l64-asan-000000000000000/build/gfx/thebes/gfxUserFontSet.cpp:891:0
.
.
.
0x603000128cb0 is located 0 bytes inside of 32-byte region [0x603000128cb0,0x603000128cd0)
freed by thread T0 (Web Content) here:
    #0 0x471721 in free _asan_rtl_:0
    #1 0x7f3695877913 in ~OpenTypeGASP /builds/slave/m-cen-l64-asan-000000000000000/build/obj-firefox/gfx/ots/src/../../../dist/include/mozilla/mozalloc.h:232:0
    #2 0x7f3695877913 in ots::ots_gasp_parse(ots::OpenTypeFile*, unsigned char const*, unsigned long) /builds/slave/m-cen-l64-asan-000000000000000/build/gfx/ots/src/gasp.cc:36:0
    #3 0x7f36958d8955 in (anonymous namespace)::ProcessGeneric(ots::OpenTypeFile*, unsigned int, ots::OTSStream*, unsigned char const*, unsigned long, std::vector<(anonymous namespace)::OpenTypeTable, std::allocator<(anonymous namespace)::OpenTypeTable> > const&, ots::Buffer&) /builds/slave/m-cen-l64-asan-000000000000000/build/gfx/ots/src/ots.cc:590:0
    #4 0x7f36958d0411 in (anonymous namespace)::ProcessTTF(ots::OpenTypeFile*, ots::OTSStream*, unsigned char const*, unsigned long) /builds/slave/m-cen-l64-asan-000000000000000/build/gfx/ots/src/ots.cc:237:0
    #5 0x7f36958cd7a5 in ots::OTSContext::Process(ots::OTSStream*, unsigned char const*, unsigned long) /builds/slave/m-cen-l64-asan-000000000000000/build/gfx/ots/src/ots.cc:824:0
    #6 0x7f36902d64f3 in gfxUserFontEntry::SanitizeOpenTypeData(unsigned char const*, unsigned int, unsigned int&, gfxUserFontType) /builds/slave/m-cen-l64-asan-000000000000000/build/gfx/thebes/gfxUserFontSet.cpp:253:0
.
.
.
Severity: normal → critical
Component: General → Graphics: Text
Assignee: nobody → jdaggett
This is a macro expansion bug.

The macro callsite here uses a struct member var 'gasp->version':

This is a macro expansion bug.

The macro callsite here uses a struct member var 'gasp->version':

  if (gasp->version > 1) {
    // Lots of Linux fonts have bad version numbers...
    uint16_t version = gasp->version;
    DROP_THIS_TABLE("bad version: %u", version);
    return true;
  }

But the macro expands to code that deletes the struct before it's used:

#define DROP_THIS_TABLE(...) \
  do { \
    delete file->gasp; \
    file->gasp = 0; \
    OTS_FAILURE_MSG_(file, TABLE_NAME ": " __VA_ARGS__); \
    OTS_FAILURE_MSG("Table discarded"); \
  } while (0)



  if (gasp->version > 1) {
    // Lots of Linux fonts have bad version numbers...
    uint16_t version = gasp->version;
    DROP_THIS_TABLE("bad version: %u", version);
    return true;
  }

But the macro expands to code that deletes the struct before it's used:

#define DROP_THIS_TABLE(...) \
  do { \
    delete file->gasp; \
    file->gasp = 0; \
    OTS_FAILURE_MSG_(file, TABLE_NAME ": " __VA_ARGS__); \
    OTS_FAILURE_MSG("Table discarded"); \
  } while (0)
Checking for DROP_THIS_TABLE across the OTS code shows that the same issue is present in several other files as well; they all need to be fixed similarly.

In general, though, I think I'd prefer to fix this by rearranging the contents of the macro so that it outputs the messages before deleting the table. That way it's not such a footgun, and the callsites won't need to be audited/fixed.

Also, OTS issues should be reported upstream, please: https://github.com/khaledhosny/ots/.
Flags: sec-bounty?
John, can you suggest a security rating for this bug?
I agree with comment #3, will just do so.
Attachment #8522706 - Flags: review?(jfkthame)
(In reply to Al Billings [:abillings] from comment #4)
> John, can you suggest a security rating for this bug?

Sorry, no idea what to suggest here. Jonathan?
Flags: needinfo?(jfkthame)
(In reply to John Daggett (:jtd) from comment #7)
> (In reply to Al Billings [:abillings] from comment #4)
> > John, can you suggest a security rating for this bug?
> 
> Sorry, no idea what to suggest here. Jonathan?

Marking this as sec-low. AFAICS, there's no actual security risk here. ASAN is right that we read a value from a block that's been freed, but the only thing we do with it is include the value in a dev console message through a "%u" format specifier. That's pretty harmless.
Flags: needinfo?(jfkthame)
Keywords: sec-low
Assignee: jdaggett → jfkthame
Status: NEW → ASSIGNED
Attachment #8523793 - Flags: review?(jdaggett) → review+
As a sec-low, this isn't bounty eligible so I'm minusing it now.
Flags: sec-bounty? → sec-bounty-
https://hg.mozilla.org/mozilla-central/rev/47f88e6ae34c
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Group: core-security
Whiteboard: [adv-main36+]
Alias: CVE-2015-0823
Flags: sec-bounty-hof+
You need to log in before you can comment on or make changes to this bug.