After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 639882 - Heap corruption in font parsing with FreeType2 backend
Heap corruption in font parsing with FreeType2 backend
Status: RESOLVED FIXED
Product: pango
Classification: Platform
Component: general
1.28.x
Other Linux
: Normal normal
: ---
Assigned To: pango-maint
pango-maint
Depends on:
Blocks:
 
 
Reported: 2011-01-18 19:38 UTC by Kees Cook
Modified: 2011-03-19 18:12 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Untested patch (1.27 KB, patch)
2011-01-21 20:45 UTC, Behdad Esfahbod
none Details | Review

Description Kees Cook 2011-01-18 19:38:06 UTC
As reported by Dan Rosenberg to Ubuntu in:
https://bugs.launchpad.net/ubuntu/+source/pango1.0/+bug/696616


When used with FreeType2 as a backend, Pango is vulnerable to heap corruption when rendering malformed fonts. The vulnerability occurs in pango_ft2_font_render_box_glyph() in pango/pangoft2-render.c. A buffer is malloc'd with size box->bitmap.rows * box->bitmap.pitch. Subsequently, 0xff is written at offsets into this buffer without checking that these offsets fall within the buffer's boundaries, leading to heap corruption.

I tested this against Lucid (Pango 1.28.0) and upstream (Pango 1.28.3).

I've attached a fuzzed version of the FreeSerif TrueType font ("crash.ttf") that can be used to reproduce this corruption as follows, using the test-mixed.txt file included in the pango-view directory of the source tree (also attached):

# cp /usr/share/fonts/truetype/freefont/FreeSerif.ttf /usr/share/fonts/truetype/freefont/FreeSerif.ttf.bak
# cp crash.ttf /usr/share/fonts/truetype/freefont/FreeSerif.ttf
# pango-view --backend=ft2 --font=FreeSerif test-mixed.txt
*** glibc detected *** pango-view: malloc(): memory corruption: 0x000000000116cfa0 ***
======= Backtrace: =========
...
Comment 2 Behdad Esfahbod 2011-01-20 19:59:35 UTC
Can you patch it to use g_malloc_n?
Comment 3 Josh Bressers 2011-01-21 15:34:00 UTC
I've spent some time investigating this. The code in question seems to go pretty far back, hitting the exploit on some version looks tricky though.

Behdad, does something using Pango have to use the PangoFT2FontMap type, or can Pango load FT2 fonts without explicitly asking for it?
Comment 4 Behdad Esfahbod 2011-01-21 20:05:49 UTC
Josh, the only uses of PangoFT2FontMap I can think of are older versions of GIMP and Inkscape.  It's NOT used by any version of GTK+ or Firefox.
Comment 5 Josh Bressers 2011-01-21 20:34:11 UTC
I see a couple of errors in this code. I'm not familiar enough with it to know how to properly fix it though.

Looking at version 1.28.3 in pangoft2-render.c

In the testcase pango_ft2_font_render_box_glyph() gets called with a width of 663 and height of 20.

If you look at line 126, we end up with a buffer of size (height * height), or 400 with the testcase.

On line 133, we have a for loop that uses width as the upper bound. So we end up writing 0xff well past our 400 byte buffer (up to around 264 bytes past the end of the buffer). I'm not sure if the answer is to fix that loop, or allocate a bigger buffer.

On line 144, we set offset2 to (width - 2 - j), so if we make it this far, we're going to be writing 0xff well beyond the buffer. The for loop on line 145 seems to stay within bounds (presuming a sane offset2 variable).

On line 159, we have a similar issue as above. We're using the width variable to calculate our offset.


I'm not sure what's right, but whatever fix is used, integer overflow protection should probably also be added to the g_malloc0 call.
Comment 6 Behdad Esfahbod 2011-01-21 20:45:30 UTC
Created attachment 179002 [details] [review]
Untested patch

Please test.
Comment 7 Josh Bressers 2011-01-21 23:57:18 UTC
With the above patch applied, pango-view no longer crashes for me. I've not done any other testing with respect to ensuring it doesn't break any other fonts.
Comment 8 Vincent Untz 2011-02-17 15:25:58 UTC
Behdad: should this go in? :-) Just wondering whether I can take the patch for stable distros...
Comment 9 Behdad Esfahbod 2011-02-17 15:50:45 UTC
Yes.  Committing.
Comment 10 Behdad Esfahbod 2011-02-17 16:20:12 UTC
Committed.
Comment 11 Pacho Ramos 2011-03-12 17:31:43 UTC
Could this be also committed to 1.28 branch? Thanks
Comment 12 Pacho Ramos 2011-03-19 18:12:41 UTC
This will also add a dep on glib-2.24 due:
http://library.gnome.org/devel/glib/unstable/glib-Memory-Allocation.html#g-malloc0-n