|
|
FreeType 2.5.3 Mac font parsing heap-based buffer overflow due to integer signedness problems | |||||||
| Project Member Reported by mjurczyk@google.com, Nov 5 2014 | Back to list | |||||||
In the freetype/src/base/ftobjs.c file, we can find multiple auxiliary functions for handling uncommon or exotic font formats. One such function is "Mac_Read_POST_Resource", which heavily operates on user-supplied data:
1586: FT_Long len;
1587: FT_Long pfb_len, pfb_pos, pfb_lenpos;
1588: FT_Long rlen, temp;
...
1628: if ( FT_READ_LONG( rlen ) )
1629: goto Exit;
...
1676: if ( pfb_pos > pfb_len || pfb_pos + rlen > pfb_len )
1677: goto Exit2;
1678:
1679: error = FT_Stream_Read( stream, (FT_Byte *)pfb_data + pfb_pos, rlen );
1680: if ( error )
1681: goto Exit2;
1682: pfb_pos += rlen;
On 32-bit builds, the "rlen" variable is a fully controlled signed integer. While the if() statement in line 1676 attempts to perform bounds checking, all variables included in the expressions are of signed type, resulting in both comparisons being signed, too. If the value of "rlen" is negative, the check is bypassed, and the value is passed as the last parameter to FT_Stream_Read, which expects an unsigned count (ftstream.h):
367: /* read bytes from a stream into a user-allocated buffer, returns an */
368: /* error if not all bytes could be read. */
369: FT_BASE( FT_Error )
370: FT_Stream_Read( FT_Stream stream,
371: FT_Byte* buffer,
372: FT_ULong count );
As a result, the function attempts to read an inadequately large number of bytes into a heap-based "pfb_data" buffer, thus resulting in memory corruption. This is illustrated by the attached example proof-of-concept sample (2.ttf), which triggers the following AddressSanitizer output:
=================================================================
==11824== ERROR: AddressSanitizer: heap-buffer-overflow on address 0xf5d3eeeb at pc 0xf5fd9ad8 bp 0xffc84fa8 sp 0xffc84f9c
WRITE of size 1 at 0xf5d3eeeb thread T0
#0 0xf5fd9ad7 in FT_Stream_ReadAt freetype2/src/base/ftstream.c:145
#1 0xf5fd98a1 in FT_Stream_Read freetype2/src/base/ftstream.c:114
#2 0xf5fc3c5e in Mac_Read_POST_Resource freetype2/src/base/ftobjs.c:1679
#3 0xf5fc443f in IsMacResource freetype2/src/base/ftobjs.c:1811
#4 0xf5fc4ad9 in IsMacBinary freetype2/src/base/ftobjs.c:1888
#5 0xf5fc50a8 in load_mac_face freetype2/src/base/ftobjs.c:2003
#6 0xf5fc59fe in FT_Open_Face freetype2/src/base/ftobjs.c:2165
#7 0xf5fc24ff in FT_New_Face freetype2/src/base/ftobjs.c:1254
#8 0x804b5a8 in get_face ft2demos-2.5.3/src/ftbench.c:705
#9 0x804bc64 in main ft2demos-2.5.3/src/ftbench.c:924
0xf5d3eeeb is located 2862 bytes to the right of 236477-byte region [0xf5d04800,0xf5d3e3bd)
allocated by thread T0 here:
#0 0xf61cf854 (/usr/lib32/libasan.so.0+0x16854)
#1 0xf5fb6df7 in ft_alloc freetype2/builds/unix/ftsystem.c:102
#2 0xf5fded74 in ft_mem_qalloc freetype2/src/base/ftutil.c:76
#3 0xf5fdebde in ft_mem_alloc freetype2/src/base/ftutil.c:55
#4 0xf5fc35de in Mac_Read_POST_Resource freetype2/src/base/ftobjs.c:1609
#5 0xf5fc443f in IsMacResource freetype2/src/base/ftobjs.c:1811
#6 0xf5fc4ad9 in IsMacBinary freetype2/src/base/ftobjs.c:1888
#7 0xf5fc50a8 in load_mac_face freetype2/src/base/ftobjs.c:2003
#8 0xf5fc59fe in FT_Open_Face freetype2/src/base/ftobjs.c:2165
#9 0xf5fc24ff in FT_New_Face freetype2/src/base/ftobjs.c:1254
#10 0x804b5a8 in get_face ft2demos-2.5.3/src/ftbench.c:705
#11 0x804bc64 in main ft2demos-2.5.3/src/ftbench.c:924
#12 0xf5e13a82 (/lib/i386-linux-gnu/libc.so.6+0x19a82)
SUMMARY: AddressSanitizer: heap-buffer-overflow freetype2/src/base/ftstream.c:145 FT_Stream_ReadAt
Shadow bytes around the buggy address:
0x3eba7d80: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x3eba7d90: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x3eba7da0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x3eba7db0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x3eba7dc0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x3eba7dd0: fa fa fa fa fa fa fa fa fa fa fa fa fa[fa]fa fa
0x3eba7de0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x3eba7df0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x3eba7e00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x3eba7e10: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x3eba7e20: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
Addressable: 00
Partially addressable: 01 02 03 04 05 06 07
Heap left redzone: fa
Heap righ redzone: fb
Freed Heap region: fd
Stack left redzone: f1
Stack mid redzone: f2
Stack right redzone: f3
Stack partial redzone: f4
Stack after return: f5
Stack use after scope: f8
Global redzone: f9
Global init order: f6
Poisoned by user: f7
ASan internal: fe
==11824== ABORTING
Please note that this problem is very similar to https://savannah.nongnu.org/bugs/?30658, and is in fact caused by the fact that the fix introduced back then is not sufficient to prevent all types of attacks.
Project Member
Comment 1
by
mjurczyk@google.com,
Nov 5 2014
,
Nov 5 2014
,
Nov 26 2014
The developer tried to fix this in http://git.savannah.gnu.org/cgit/freetype/freetype2.git/commit/?id=35252ae9aa1dd9343e9f4884e9ddb1fee10ef415, but the vulnerability can still be triggered for very large files (> 2GB). I'm pinging the developer to complement the fix with an additional check.
,
Nov 26 2014
In fact, when https://code.google.com/p/google-security-research/issues/detail?id=153 is properly fixed, the issue should be resolved. I'm marking this one as fixed, then.
,
Jan 26 2015
All fixed by upstream: FreeType 2.5.5 2014-12-30 FreeType 2.5.5 has been released. This is a minor bug fix release: All users of PCF fonts should update, since version 2.5.4 introduced a bug that prevented reading of such font files if not compressed. FreeType 2.5.4 2014-12-06 FreeType 2.5.4 has been released. All users should upgrade due to another fix for vulnerability CVE-2014-2240 in the CFF driver. The library also contains a new round of patches for better protection against malformed fonts. The main new feature, which is also one of the targets mentioned in the pledgie roadmap below, is auto-hinting support for Devanagari and Telugu, two widely used Indic scripts. A more detailed description of the remaining changes and fixes can be found here.
,
Feb 24 2015
Information in this issue does not seem to be correct. This is the version of the code form the time of the report (early Nov 2014): http://git.savannah.gnu.org/cgit/freetype/freetype2.git/tree/src/base/ftobjs.c?id=bbd8313#n1574 Description above quotes the following code: 1628: if ( FT_READ_LONG( rlen ) ) 1629: goto Exit; ... 1676: if ( pfb_pos > pfb_len || pfb_pos + rlen > pfb_len ) 1677: goto Exit2; 1678: 1679: error = FT_Stream_Read( stream, (FT_Byte *)pfb_data + pfb_pos, rlen ); It does not quote another important code in between the above lines: 1639 /* the flags are part of the resource, so rlen >= 2. */ 1640 /* but some fonts declare rlen = 0 for empty fragment */ 1641 if ( rlen > 2 ) 1642 rlen -= 2; 1643 else 1644 rlen = 0; Hence 'If the value of "rlen" is negative, the check is bypassed' should never happen, as rlen can not be negative on line 1676. The rlen value in the attached test case is 0x7fffffff, which is positive. AFAICS, this is not a separate issue, but a different test case for the integer overflow problems described in issue #153 .
,
Feb 24 2015
The part that probably can be considered to not be part of issue #153 is handling of temp. As that is signed, it can reduce pfb_len counter value without actually making it wrap around. When it's re-read as rlen, excessively long read/write in FT_Stream_Read() is avoided by the rlen check mentioned in comment #6 above, but having it tagged as comment would be another way.
,
Feb 25 2015
,
Feb 25 2015
@thoger, I have looked at the source code again and I believe you are right - it was not possible for a negative "rlen" to slip through up to lines 1676 - 1679 because of the if() statement in between, so the problem is not really as originally described. However, there were still multiple integer signedness handling problems in the quoted fragments of code, which might or might not have been mitigated/prevented by fixes from issue 153 . The intent in creating two issues was to separate the wrap-around integer overflows ( issue 153 ) and the signedness problems in sanity checking performed in the code (this issue). It has also resulted in converting the local variables in the function to unsigned types and introducing more in-depth bounds checking, so I am leaving the bug as is.
,
Apr 20 2015
|
||||||||
| ► Sign in to add a comment | ||||||||
