Last Comment Bug 480134 - (CVE-2009-1194) Heap/integer overflows during font glyph rendering reachable in FF and Camino (libpango, CoreGraphics)
(CVE-2009-1194)
: Heap/integer overflows during font glyph rendering reachable in FF and Camino...
Status: RESOLVED FIXED
[sg:critical?] libpango and ripc issu...
: fixed1.9.1, verified1.9.0.12
Product: Core
Classification: Components
Component: Graphics (show other bugs)
: 1.9.0 Branch
: x86 Mac OS X
: P2 critical (vote)
: ---
Assigned To: Vladimir Vukicevic [:vlad] [:vladv]
:
Mentors:
http://static.dataspill.org/glyph
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-02-25 09:10 PST by Will Drewry
Modified: 2013-03-31 15:58 PDT (History)
20 users (show)
vladimir: blocking1.9.1+
dveditz: blocking1.9.0.12+
dveditz: wanted1.9.0.x+
stransky: wanted1.8.1.x-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch for pango_glyph_string_set_size (882 bytes, patch)
2009-02-26 00:03 PST, Karl Tomlinson (ni?:karlt)
no flags Details | Diff | Review
patch for pango_glyph_string_set_size (with more context) (1.22 KB, patch)
2009-02-26 00:25 PST, Karl Tomlinson (ni?:karlt)
no flags Details | Diff | Review
patch for safe abort in pango_glyph_string_set_size (1.23 KB, patch)
2009-02-26 14:10 PST, Karl Tomlinson (ni?:karlt)
no flags Details | Diff | Review
apply OSX limit to all OSX versions (728 bytes, patch)
2009-04-06 13:06 PDT, Vladimir Vukicevic [:vlad] [:vladv]
roc: review+
vladimir: approval1.9.1+
dveditz: approval1.9.0.12+
Details | Diff | Review

Description Will Drewry 2009-02-25 09:10:03 PST
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.
Comment 1 Jonathan Kew (:jfkthame) 2009-02-25 09:58:45 PST
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
Comment 2 Will Drewry 2009-02-25 12:00:59 PST
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.
Comment 3 Daniel Veditz [:dveditz] 2009-02-25 12:10:11 PST
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.
Comment 4 Vladimir Vukicevic [:vlad] [:vladv] 2009-02-25 15:34:35 PST
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..
Comment 5 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2009-02-25 16:59:27 PST
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.
Comment 6 Karl Tomlinson (ni?:karlt) 2009-02-25 17:03:44 PST
Bug 357637 comment 59 also mentions the integer overflow in Pango, which may be
the same issue as bug 430127.
Comment 7 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2009-02-25 17:10:10 PST
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.
Comment 8 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2009-02-25 17:29:27 PST
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.
Comment 9 Karl Tomlinson (ni?:karlt) 2009-02-25 18:58:27 PST
(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.
Comment 10 Karl Tomlinson (ni?:karlt) 2009-02-26 00:03:22 PST
Created attachment 364264 [details] [diff] [review]
patch for pango_glyph_string_set_size
Comment 11 Karl Tomlinson (ni?:karlt) 2009-02-26 00:25:29 PST
Created attachment 364268 [details] [diff] [review]
patch for pango_glyph_string_set_size (with more context)
Comment 12 Karl Tomlinson (ni?:karlt) 2009-02-26 14:10:05 PST
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.
Comment 13 Behdad Esfahbod 2009-03-02 00:30:50 PST
I'm committed a similar patch to Pango upstream now.  Will be in 1.24.
Comment 14 Will Drewry 2009-03-02 08:53:34 PST
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
Comment 15 Vladimir Vukicevic [:vlad] [:vladv] 2009-03-02 16:04:12 PST
We should at least put in some barriers like what roc described for 1.9.1 for this.
Comment 16 Stuart Morgan 2009-03-02 20:06:54 PST
(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?)
Comment 17 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2009-03-02 20:26:21 PST
(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.
Comment 18 Stuart Morgan 2009-03-03 07:25:36 PST
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).
Comment 19 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2009-03-03 11:53:51 PST
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 Behdad Esfahbod 2009-03-04 02:39:08 PST
(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
Comment 21 Vladimir Vukicevic [:vlad] [:vladv] 2009-03-10 04:18:13 PDT
A similar issue to the CG thing here is showing up in bug 482128.
Comment 22 Vladimir Vukicevic [:vlad] [:vladv] 2009-03-31 11:53:25 PDT
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?
Comment 23 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2009-03-31 12:21:44 PDT
(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.
Comment 24 Will Drewry 2009-03-31 12:42:05 PDT
> > - 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).
Comment 25 Vladimir Vukicevic [:vlad] [:vladv] 2009-04-06 13:06:07 PDT
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.
Comment 26 Vladimir Vukicevic [:vlad] [:vladv] 2009-04-07 12:25:16 PDT
Comment on attachment 371294 [details] [diff] [review]
apply OSX limit to all OSX versions

Mac chunk: http://hg.mozilla.org/mozilla-central/rev/4256bc50a5db
Comment 27 Vladimir Vukicevic [:vlad] [:vladv] 2009-04-08 13:38:02 PDT
Let's call this fixed -- we put in the Mac bandaid, and the linux fixes will come from pango updates.  Reopen if anyone disagrees.
Comment 28 Daniel Veditz [:dveditz] 2009-05-07 15:49:23 PDT
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?
Comment 29 Daniel Veditz [:dveditz] 2009-05-28 11:17:51 PDT
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
Comment 30 Daniel Veditz [:dveditz] 2009-06-24 23:25:01 PDT
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
Comment 31 Al Billings [:abillings] 2009-06-30 14:50:09 PDT
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.
Comment 32 Daniel Veditz [:dveditz] 2009-07-06 17:44:40 PDT
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 Martin Stránský 2009-07-08 06:32:02 PDT
1.8 looks safe.
Comment 34 Al Billings [:abillings] 2009-07-09 09:23:20 PDT
Based on Dan's comments, marking this as verified for 1.9.0.12.
Comment 35 Josh Bressers 2009-07-09 10:10:37 PDT
This is CVE-2009-1194

Note You need to log in before you can comment on or make changes to this bug.