Bug 480134 (CVE-2009-1194)

Heap/integer overflows during font glyph rendering reachable in FF and Camino (libpango, CoreGraphics)

RESOLVED FIXED

Status

()

Core
Graphics
P2
critical
RESOLVED FIXED
9 years ago
4 years ago

People

(Reporter: Will Drewry, Assigned: vlad)

Tracking

({fixed1.9.1, verified1.9.0.12})

1.9.0 Branch
x86
Mac OS X
fixed1.9.1, verified1.9.0.12
Points:
---
Bug Flags:
blocking1.9.1 +
blocking1.9.0.12 +
wanted1.9.0.x +
wanted1.8.1.x -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:critical?] libpango and ripc issue; multi-product issue (oCERT-2009-001), URL)

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

9 years ago
User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.0.6) Gecko/2009011912 Firefox/3.0.6
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.0.6) Gecko/2009011912 Firefox/3.0.6

Both Firefox and Camino are exposed to a font rendering integer overflow which exists independently in libpango and in Apple's CoreGraphics (ripc).

I have reported the bug upstream to Apple, but they have no timeline to fix. (Yes, this affects Safari too..)

The issue is in URL bar rendering.  If a very long document.location is set via javascript, the integer overflow is reachable.  The files at the above location show this bug for each.  

The affected code in pango is in pango_glyph_string_set_size:
An overflow check when doubling the size neglects the overflow possible on the subsequent allocation:
  string->glyphs = g_realloc (string->glyphs, string->space * sizeof (PangoGlyphInfo));

I don't have a live exploit, but this looks exploitable.  I have not yet gotten this reported upstream to libpango.  It is pending on their mailing list and I've received no response.

Because of their delay and Apple's delay, I thought it would make the most sense to put a maximum limit on the document.location.

This will be referenced with advisory oCERT-2009-001.




Reproducible: Always

Steps to Reproduce:
1. Goto a pango crasher in http://static.dataspill.org/glyph/
   E.g., http://static.dataspill.org/glyph/pango_2.html
2. Crash.

Depending on how the overflow is handled, you can get it to proceed without a crash.

(Note, I've been tinkering with the crashers so there's nothing special about the different versions, just a few tweaks.)
Actual Results:  
In camino, it is often a NULL deref.  I can get a few other random stack-addresses visible, but I haven't done a thorough analysis on the Apple libraries to know.

In ff, pango exposes the num_glyphs after a successful underallocation.  If this is used for writing to the glyph array, it will result in heap corruption.

Expected Results:  
Maximum URL length and/or integer overflow checks in libpango/ripc would stop this.

Please let me know if I can help coordinate anything.  I can be reached at my personal address: redpig@dataspill.org or at my oCERT addr: redpig@ocert.org.

I will be pursuing getting this upstream to pango, but if you have any contacts that can help, that'd be great.

In addition, if you have some idea for Camino fixes, if you plan to release the crashers, please let me know in advance.  I would like to negotiate some embargo date with Apple if possible, but who knows.
Component: General → GFX: Thebes
Product: Firefox → Core
QA Contact: general → thebes
Version: unspecified → 1.9.0 Branch
Flags: blocking1.9.1?
Flags: blocking1.9.0.8?
Whiteboard: [sg:investigate] libpango and ripc issue
Crashed for me after an allocation failure (std::bad_alloc exception) while building a gfxTextRun, presumably out-of-memory.

Another test crashed with memory allocation failure in nsAString_internal while trying to modify a string with length over 100,000,000 characters(!).

This suggests that allowing a script to create such huge strings is a more general problem, not just an issue for drawing the URL. The exact point where the out-of-memory crash will occur may be difficult to predict, but it's clear that we don't always handle allocation failure well.

When I tried this in Safari just now, it actually crashed in WebCore::KURL::parse, rather than in the font code (although I can easily believe that is also possible).

I tried this on Windows as well (Minefield trunk build); it appeared to hang for some time, and presented an "unresponsive script" message, but allowing it to continue did eventually succeed. It hit several assertions along the way, though:

###!!! ASSERTION: CopyItemSplitOversize() failed: 'NS_SUCCEEDED(nrs)', file
c:/mozdev/gfx/thebes/src/gfxWindowsFonts.cpp, line 2204
###!!! ASSERTION: UniscribeItem is too big, ScriptShape() will fail!:
'mMaxGlyphs < 65535', file c:/mozdev/gfx/thebes/src/gfxWindowsFonts.cpp, line 1638
###!!! ASSERTION: Failed to shape, twice -- we should never hit this:'SUCCEEDED
(rv)', file c:/mozdev/gfx/thebes/src/gfxWindowsFonts.cpp, line 2494
###!!! ASSERTION: UniscribeItem is too big, ScriptShape() will fail!:
'mMaxGlyphs < 65535', file c:/mozdev/gfx/thebes/src/gfxWindowsFonts.cpp, line 1638
(Reporter)

Comment 2

9 years ago
The WebCore bug when I hit it was a NULL deref.  The allocation appears to fail and they don't fail gracefully. I guess I should file a webkit bug too - not sure if Apple propagated my report.
Should this bug be split into an Apple branch and a Pango branch? Or should we keep it together and fix the assertions noted in comment 1 (mMaxGlyphs < 65535) to avoid triggering either problem. 64K glyphs in one run ought to be enough for anyone.
Flags: blocking1.9.0.8? → wanted1.9.0.x+
Status: UNCONFIRMED → NEW
Ever confirmed: true
I'd say let's just keep them together into this bug, and just clamp down on the number of glyphs -- probably much less than 64k, 16k should be fine for one run.  roc, does that sound ok?  I don't really see us doing a 16k-glyph run in any real world usage..
We certainly can have more than 16k glyphs in real-world usage. A single very large paragraph of white-space:normal text will do it.
Bug 357637 comment 59 also mentions the integer overflow in Pango, which may be
the same issue as bug 430127.
I think we should safely handle textrun creation failures due to OOM. This shouldn't be too hard.

We should also cap the maximum length of strings we pass to platform font engines. We already have code that caps strings passed to ATSUI at 500 characters (or less, when necessary, to work around 32K coordinate limits); see gfxAtsuiFontGroup::GuessMaximumStringLength. It doesn't fail, it just breaks up the incoming strings for separate processing and then glues the resulting textruns back together. The incoming text is broken up quite intelligently, generally at space boundaries so we're unlikely to disturb shapers. We should generalize that to all platforms and limit the text to something sensible (say 1K chars) no matter what.
One thing to keep in mind is that a malicious downloaded font might be able to associate a very large number of glyphs with a single character. Someone who knows a lot about Opentype and AAT might be able to figure out how large that number is.

So we should probably cap the passed-down string length, say to <1K, but also check the number of glyphs returned and bail if it's too large, say >32K.
(In reply to comment #8)
> also check the number of glyphs returned and bail if it's too large, say >32K.

The problem here, with Pango, is that we can't check the number of glyphs until after pango_shape has returned, at which point even the number of glyphs may have been overwritten by untrusted data.

It seems that this needs to be fixed in the library.

If we can't rely on the library being fixed, pango_glyph_string_set_size is a dynamic symbol, so Mozilla can provide its own implementation.  PangoGlyphString::space is meant to be private, but I haven't come up with any better ideas.
Created attachment 364264 [details] [diff] [review]
patch for pango_glyph_string_set_size
Attachment #364264 - Flags: review?(mozilla)
Created attachment 364268 [details] [diff] [review]
patch for pango_glyph_string_set_size (with more context)
Attachment #364264 - Attachment is obsolete: true
Attachment #364268 - Flags: review?(mozilla)
Attachment #364264 - Flags: review?(mozilla)
Attachment #364268 - Flags: review?(mozilla)
Created attachment 364394 [details] [diff] [review]
patch for safe abort in pango_glyph_string_set_size 

Callers don't check the new value of num_glyphs, so I think the best thing to
do is to abort if allocation will not be possible.  This is consistent with
what g_realloc will do on failure anyway.

I'm doubting this particular overflow is likely to be exploitable with current
Pango code (but the patch would make things correct).

This overflow would only happen on 32-bit systems, so I'll consider 32-bit
systems here.

Will, have you seen a SIGSEGV with Pango?
On 32-bit or 64-bit systems?

sizeof(PangoGlyphInfo) is 0x14, so the smallest new_len passed to
pango_glyph_string_set_size that will cause an overflow is 0x08000001.

However, the only callers to pango_glyph_string_set_size that subsequently
write to the PangoGlyphString and don't increase the new_len gradually are
pango_ot_buffer_output and _pango_shape_shape.

For callers that increase new_len gradually, a value of
0x04000000 < new_len <= 0x08000000 will try to g_realloc
0x08000000 * 0x14 = 0xa0000000 (3 GB) which seems very unlikely to succeed,
and failure results in an abort.

pango_ot_buffer_output will set new_len to HB_Buffer::in_length.  But it looks
like this will only be a large as a successful allocation of
n * sizeof(HB_GlyphItemRec).  HB_GlyphItemRec seems to need 0x12 bytes, which
I assume would align to 0x14, so an allocation of at least 0x08000001 * 0x14 =
0xa0000014 (>3GB) must succeed before new_len will be >= 0x08000001 in
pango_glyph_string_set_size.  (The size of the HB_GlyphItemRec buffer is
increased gradually.)

_pango_shape_shape is only called from pango-layout, which Mozilla only uses
through GTK (not through our text code), and only when the text has
PANGO_ATTR_SHAPE.
Attachment #364268 - Attachment is obsolete: true
Attachment #364394 - Flags: review?(mozilla)

Comment 13

9 years ago
I'm committed a similar patch to Pango upstream now.  Will be in 1.24.
(Reporter)

Comment 14

9 years ago
Hi Karl --  Thanks for the thorough analysis/explanation.  I completely admit I've been lazy on this one, and most of my time was spent looking at the apple side of things (as that is what led to the discovery).  I've only had one SIGSEGV with FF, and I'm pretty sure it is unrelated [and, so far, unreproducible].

So, in short:
- FF+pango: allocation crash/dos only(*)
- Camino+ripc: sigsegvs with some ability to control; possibly exploitable


[*] Note, it is possible to also dos ff using large runs of "A\n" in a html node as it will add/remove the nodes over and over again up to whatever huge string size without triggering the javascript timeout: http://static.dataspill.org/glyph/pango_mac_alloc_loop.html


Behdad -- Awesome! I'm glad to see the patch heading upstream.  When is 1.24 planned to go out?

thanks!
will
We should at least put in some barriers like what roc described for 1.9.1 for this.
Flags: blocking1.9.1? → blocking1.9.1+
Priority: -- → P2

Comment 16

9 years ago
(In reply to comment #14)
> So, in short:
> - FF+pango: allocation crash/dos only(*)
> - Camino+ripc: sigsegvs with some ability to control; possibly exploitable

Just to make sure I'm understanding correctly: Camino would need to cap the length of every single string that might ever come from Core and end up being displayed in the UI? And doing so removes exploitability even if other Core crashes related to ginormous strings remain unfixed on the 1.9.0 branch?

(Or alternately, Camino could presumably wait for an OS update... do you know specifically what versions of the OS are vulnerable?)
(In reply to comment #16)
> (In reply to comment #14)
> > So, in short:
> > - FF+pango: allocation crash/dos only(*)
> > - Camino+ripc: sigsegvs with some ability to control; possibly exploitable
> 
> Just to make sure I'm understanding correctly: Camino would need to cap the
> length of every single string that might ever come from Core and end up being
> displayed in the UI? And doing so removes exploitability even if other Core
> crashes related to ginormous strings remain unfixed on the 1.9.0 branch?

No. There is nothing Camino-specific here other than that the bug reporter has failed to reproduce their particular bug in Firefox for some reason. We can mitigate this in Core.
Attachment #364394 - Flags: review?(mozilla)

Comment 18

9 years ago
If part of the bug is that Apple libraries will crash when given incredibly long strings, which seems to be what comment 0 is saying, then how does comment 7 address the entire problem for Camino? There is no direct Core-to-platform-font-engine interface in the case of the URL bar; Core gives Camino the string, and Camino gives it to the OS to render (through native controls).
Ah, that's true. You can't trust Apple's text drawing APIs so whenever you use them directly, you'll have to do something. Core can't help there.

Comment 20

9 years ago
(In reply to comment #14)

> Behdad -- Awesome! I'm glad to see the patch heading upstream.  When is 1.24
> planned to go out?

In a week or two.
 
> thanks!
> will
A similar issue to the CG thing here is showing up in bug 482128.
So it sounds like the Core fixes here are:

- On Mac, make 500-char CG/Quartz limit apply to all OS X versions
- On Linux, no Core fixes, just depend on the fix being in pango 1.24

Right?
(In reply to comment #22)
> - On Mac, make 500-char CG/Quartz limit apply to all OS X versions

Yep

> - On Linux, no Core fixes, just depend on the fix being in pango 1.24

You mean relying on distros to backport the fix to all their supported releases? OK I guess.
(Reporter)

Comment 24

9 years ago
> > - On Linux, no Core fixes, just depend on the fix being in pango 1.24
> 
> You mean relying on distros to backport the fix to all their supported
> releases? OK I guess.

Since there was no fanfare for the pango 1.24 release and FF/linux won't be making any changes (it seems), I'll drop a line to various vendors soon to let them know that they may want to backport the pango fix.  It'd be nice to be able to give them the option to do so before this bug becomes public (even if they choose not to fix it).
Created attachment 371294 [details] [diff] [review]
apply OSX limit to all OSX versions

Here's the OSX patch; just apply the 500 char limit to all OSX versions.
Attachment #371294 - Flags: review?(roc)
Attachment #371294 - Flags: review?(roc) → review+
Attachment #371294 - Flags: approval1.9.1?
Comment on attachment 371294 [details] [diff] [review]
apply OSX limit to all OSX versions

Mac chunk: http://hg.mozilla.org/mozilla-central/rev/4256bc50a5db
Let's call this fixed -- we put in the Mac bandaid, and the linux fixes will come from pango updates.  Reopen if anyone disagrees.
Assignee: nobody → vladimir
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Attachment #371294 - Flags: approval1.9.1? → approval1.9.1+
Keywords: fixed1.9.1
Attachment #371294 - Flags: approval1.9.0.12?
Comment on attachment 371294 [details] [diff] [review]
apply OSX limit to all OSX versions

Any reason not to take this in 1.9.0.x?
Alias: oCERT-2009-001
Flags: blocking1.9.0.12?
Whiteboard: [sg:investigate] libpango and ripc issue → [sg:critical?] libpango and ripc issue; multi-product issue
Flags: blocking1.9.0.12? → blocking1.9.0.12+
Comment on attachment 371294 [details] [diff] [review]
apply OSX limit to all OSX versions

Approved for 1.9.0.12, a=dveditz for release-drivers
Attachment #371294 - Flags: approval1.9.0.12? → approval1.9.0.12+
Checking in gfx/thebes/src/gfxAtsuiFonts.cpp;
/cvsroot/mozilla/gfx/thebes/src/gfxAtsuiFonts.cpp,v  <--  gfxAtsuiFonts.cpp
new revision: 1.102; previous revision: 1.101
Keywords: fixed1.9.0.12
Going to http://static.dataspill.org/glyph/pango_2.html, causing the latest 1.9.0 build to peg the cpu above 90%, use over 2 GB of RAM, and become non-responsive: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.0.12pre) Gecko/2009063004 GranParadiso/3.0.12pre. I don't get an actual crash but a complete hang. This does not appear to be fixed.
I see a CPU and memory spike, but after 10-15 seconds the CPU comes back and the memory is released when I close the tab. If you've got less than the amount of memory it's trying to use you might hit swapping/performance problems, but that isn't this bug.

Comment 33

8 years ago
1.8 looks safe.
Flags: wanted1.8.1.x-
Based on Dan's comments, marking this as verified for 1.9.0.12.
Keywords: fixed1.9.0.12 → verified1.9.0.12

Comment 35

8 years ago
This is CVE-2009-1194
Alias: oCERT-2009-001 → CVE-2009-1194
Whiteboard: [sg:critical?] libpango and ripc issue; multi-product issue → [sg:critical?] libpango and ripc issue; multi-product issue (oCERT-2009-001)
Group: core-security
You need to log in before you can comment on or make changes to this bug.