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)
Core
Graphics: Text
Tracking
()
RESOLVED
FIXED
mozilla36
People
(Reporter: attekett, Assigned: jfkthame)
Details
(Keywords: csectype-uaf, reporter-external, sec-low, Whiteboard: [adv-main36+])
Attachments
(3 files)
24.13 KB,
text/html
|
Details | |
878 bytes,
patch
|
Details | Diff | Splinter Review | |
26.30 KB,
patch
|
jtd
:
review+
|
Details | Diff | Splinter Review |
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 . . .
Updated•10 years ago
|
Severity: normal → critical
Component: General → Graphics: Text
Updated•10 years ago
|
Assignee: nobody → jdaggett
Comment 1•10 years ago
|
||
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)
Assignee | ||
Comment 3•10 years ago
|
||
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/.
Updated•10 years ago
|
Flags: sec-bounty?
Comment 6•10 years ago
|
||
https://github.com/khaledhosny/ots/commit/003c62d28ae438aa8943cb31535563397f838a2c
Updated•10 years ago
|
Attachment #8522706 -
Flags: review?(jfkthame)
Comment 7•10 years ago
|
||
(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)
Assignee | ||
Comment 8•10 years ago
|
||
(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 | ||
Updated•10 years ago
|
Assignee: jdaggett → jfkthame
Status: NEW → ASSIGNED
Updated•10 years ago
|
Attachment #8523793 -
Flags: review?(jdaggett) → review+
Assignee | ||
Comment 10•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/47f88e6ae34c
Target Milestone: --- → mozilla36
Comment 11•10 years ago
|
||
As a sec-low, this isn't bounty eligible so I'm minusing it now.
Flags: sec-bounty? → sec-bounty-
Comment 12•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/47f88e6ae34c
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
status-b2g-v2.2:
--- → fixed
status-firefox36:
--- → fixed
Resolution: --- → FIXED
Updated•10 years ago
|
status-firefox-esr31:
--- → wontfix
Updated•10 years ago
|
Group: core-security
Updated•10 years ago
|
Whiteboard: [adv-main36+]
Updated•10 years ago
|
Alias: CVE-2015-0823
Updated•7 years ago
|
Keywords: csectype-uaf
Updated•4 years ago
|
Flags: sec-bounty-hof+
Updated•3 months ago
|
Keywords: reporter-external
You need to log in
before you can comment on or make changes to this bug.
Description
•