Re: Fixing CVE-2014-9674 (freetype) in wheezy
Hi Guido,
thanks for the debdiff. It looks good, except for the urgency which
you'll want to set to "high" before uploading. Once that's done, I'll
release the DSA.
Cheers,
--Seb
On Jan/24, Guido Günther wrote:
> Dear security team,
> while looking into CVEs that are fixed in Jessie and Squeeze but not yet
> in Wheezy I came across:
>
> https://security-tracker.debian.org/tracker/CVE-2014-9674
>
> Since the fix consists of several commits including a fix for
> CVE-2014-9673 (which already was fixed in the package) I pushed
> the repo I used to cp the fixes here:
>
> https://github.com/agx/freetype2/commits/debian/wheezy
>
> I'm happy about any review and and the possibility to upload this to
> security master.
>
> Cheers,
> -- Guido
> diff --git a/debian/changelog b/debian/changelog
> index afe415c..f706b95 100644
> --- a/debian/changelog
> +++ b/debian/changelog
> @@ -1,3 +1,12 @@
> +freetype (2.4.9-1.1+deb7u3) wheezy-security; urgency=medium
> +
> + * Non-maintainer upload by LTS team.
> + * CVE-2014-9674: integer overflow and heap-based buffer overflow
> + in Mac_Read_POST_Resource. The added patch also includes the fixes for
> + CVE-2014-9673 since they overlap. Closes: #777656
> +
> + -- Guido Günther <agx@sigxcpu.org> Sun, 24 Jan 2016 19:41:13 +0100
> +
> freetype (2.4.9-1.1+deb7u2) wheezy-security; urgency=high
>
> * Non-maintainer upload.
> diff --git a/debian/patches-freetype/CVE-2014-9673.patch b/debian/patches-freetype/CVE-2014-9673.patch
> deleted file mode 100644
> index 331f40f..0000000
> --- a/debian/patches-freetype/CVE-2014-9673.patch
> +++ /dev/null
> @@ -1,43 +0,0 @@
> -diff -aur freetype-2.4.9.orig/src/base/ftobjs.c freetype-2.4.9/src/base/ftobjs.c
> ---- freetype-2.4.9.orig/src/base/ftobjs.c 2012-02-11 10:29:31.000000000 +0100
> -+++ freetype-2.4.9/src/base/ftobjs.c 2015-02-19 11:27:54.271340093 +0100
> -@@ -1588,6 +1588,11 @@
> - goto Exit2;
> - if ( FT_READ_LONG( rlen ) )
> - goto Exit;
> -+ if ( rlen < 0 )
> -+ {
> -+ error = FT_Err_Invalid_Offset;
> -+ goto Exit;
> -+ }
> - if ( FT_READ_USHORT( flags ) )
> - goto Exit;
> - FT_TRACE3(( "POST fragment[%d]: offsets=0x%08x, rlen=0x%08x, flags=0x%04x\n",
> -@@ -1605,7 +1610,14 @@
> - rlen = 0;
> -
> - if ( ( flags >> 8 ) == type )
> -+ {
> -+ if ( 0x7FFFFFFFL - rlen < len )
> -+ {
> -+ error = FT_Err_Array_Too_Large;
> -+ goto Exit2;
> -+ }
> - len += rlen;
> -+ }
> - else
> - {
> - if ( pfb_lenpos + 3 > pfb_len + 2 )
> -@@ -1634,6 +1646,11 @@
> - }
> -
> - error = FT_Err_Cannot_Open_Resource;
> -+ if ( rlen > 0x7FFFFFFFL - pfb_pos )
> -+ {
> -+ error = FT_Err_Array_Too_Large;
> -+ goto Exit2;
> -+ }
> - if ( pfb_pos > pfb_len || pfb_pos + rlen > pfb_len )
> - goto Exit2;
> -
> -Nur in freetype-2.4.9/src/base: ftobjs.c~.
> diff --git a/debian/patches-freetype/CVE-2014-9674+CVE-2014-9673.diff b/debian/patches-freetype/CVE-2014-9674+CVE-2014-9673.diff
> new file mode 100644
> index 0000000..5de5b33
> --- /dev/null
> +++ b/debian/patches-freetype/CVE-2014-9674+CVE-2014-9673.diff
> @@ -0,0 +1,205 @@
> +commit c57ccea8fe7bbdc5194bf7f2bdaa3d84a788916c
> +Author: Guido Günther <agx@sigxcpu.org>
> +Date: Sun Jan 24 12:13:04 2016 +0100
> +
> + Don't use FT_ERR or FT_THROW
> +
> +commit 920aebcc8fa6ec5dfb1f9eca86598414e2363261
> +Author: suzuki toshiya <mpsuzuki@hiroshima-u.ac.jp>
> +Date: Thu Nov 27 00:20:48 2014 +0900
> +
> + * src/base/ftobj.c (Mac_Read_POST_Resource): Additional
> + overflow check in the summation of POST fragment lengths,
> + suggested by Mateusz Jurczyk <mjurczyk@google.com>.
> +
> + (cherry picked from commit cd4a5a26e591d01494567df9dec7f72d59551f6e)
> +
> +commit 8b51acd483ff65159e0af508a2d47d8f2753ad28
> +Author: suzuki toshiya <mpsuzuki@hiroshima-u.ac.jp>
> +Date: Wed Nov 26 16:39:00 2014 +0900
> +
> + * src/base/ftobjs.c (Mac_Read_POST_Resource): Insert comments
> + and fold too long tracing messages.
> +
> + (cherry picked from commit 1720e81e3ecc7c266e54fe40175cc39c47117bf5)
> +
> +commit 72e8e7cf2c4931bf31046f70db07feb4c89b72ef
> +Author: suzuki toshiya <mpsuzuki@hiroshima-u.ac.jp>
> +Date: Wed Nov 26 16:02:17 2014 +0900
> +
> + * src/base/ftobjs.c (Mac_Read_POST_Resource): Use unsigned long
> + variables to read the lengths in POST fragments. Suggested by
> + Mateusz Jurczyk <mjurczyk@google.com>.
> +
> + (cherry picked from commit 453316792fee912cfced48e9e270e9eb19892e64)
> +
> +commit f8f730dd9399d6ef5709c672a7b3fa531caececb
> +Author: suzuki toshiya <mpsuzuki@hiroshima-u.ac.jp>
> +Date: Wed Nov 26 15:52:23 2014 +0900
> +
> + Fix Savannah bug #43539.
> +
> + * src/base/ftobjs.c (Mac_Read_POST_Resource): Fix integer overflow
> + by a broken POST table in resource-fork.
> +
> + (cherry picked from commit 35252ae9aa1dd9343e9f4884e9ddb1fee10ef415)
> +
> +commit 61e33db762b54188e83eb0ceedc32503abe50db3
> +Author: suzuki toshiya <mpsuzuki@hiroshima-u.ac.jp>
> +Date: Wed Nov 26 15:43:29 2014 +0900
> +
> + Fix Savannah bug #43538.
> +
> + * src/base/ftobjs.c (Mac_Read_POST_Resource): Fix integer overflow
> + by a broken POST table in resource-fork.
> +
> + (cherry picked from commit 240c94a185cd8dae7d03059abec8a5662c35ecd3)
> +
> +commit ab20e2dc68cbcde838b25b0694e4610b0d9c8017
> +Author: suzuki toshiya <mpsuzuki@hiroshima-u.ac.jp>
> +Date: Wed Nov 26 14:36:12 2014 +0900
> +
> + * src/base/ftobjs.c (Mac_Read_POST_Resource): Avoid memory leak
> + by a broken POST table in resource-fork. Return after freeing
> + the buffered POST table when it is found to be broken.
> +
> + (cherry picked from commit 5aff85301bdce7677766fa1367c82ff41a739637)
> +diff --git a/src/base/ftobjs.c b/src/base/ftobjs.c
> +index 36ee797..3025a97 100644
> +--- a/src/base/ftobjs.c
> ++++ b/src/base/ftobjs.c
> +@@ -1544,9 +1544,9 @@
> + FT_Memory memory = library->memory;
> + FT_Byte* pfb_data = NULL;
> + int i, type, flags;
> +- FT_Long len;
> +- FT_Long pfb_len, pfb_pos, pfb_lenpos;
> +- FT_Long rlen, temp;
> ++ FT_ULong len;
> ++ FT_ULong pfb_len, pfb_pos, pfb_lenpos;
> ++ FT_ULong rlen, temp;
> +
> +
> + if ( face_index == -1 )
> +@@ -1562,11 +1562,34 @@
> + error = FT_Stream_Seek( stream, offsets[i] );
> + if ( error )
> + goto Exit;
> +- if ( FT_READ_LONG( temp ) )
> ++ if ( FT_READ_ULONG( temp ) )
> + goto Exit;
> ++
> ++ /* FT2 allocator takes signed long buffer length,
> ++ * too large value causing overflow should be checked
> ++ */
> ++ FT_TRACE4(( " POST fragment #%d: length=0x%08x\n",
> ++ i, temp));
> ++ if ( 0x7FFFFFFFUL < temp || pfb_len + temp + 6 < pfb_len )
> ++ {
> ++ FT_TRACE2(( " too long fragment length makes"
> ++ " pfb_len confused: temp=0x%08x\n", temp ));
> ++ error = FT_Err_Invalid_Offset;
> ++ goto Exit;
> ++ }
> ++
> + pfb_len += temp + 6;
> + }
> +
> ++ FT_TRACE2(( " total buffer size to concatenate %d"
> ++ " POST fragments: 0x%08x\n",
> ++ resource_cnt, pfb_len + 2));
> ++ if ( pfb_len + 2 < 6 ) {
> ++ FT_TRACE2(( " too long fragment length makes"
> ++ " pfb_len confused: pfb_len=0x%08x\n", pfb_len ));
> ++ error = FT_Err_Array_Too_Large;
> ++ goto Exit;
> ++ }
> + if ( FT_ALLOC( pfb_data, (FT_Long)pfb_len + 2 ) )
> + goto Exit;
> +
> +@@ -1586,16 +1609,30 @@
> + error = FT_Stream_Seek( stream, offsets[i] );
> + if ( error )
> + goto Exit2;
> +- if ( FT_READ_LONG( rlen ) )
> +- goto Exit;
> ++ if ( FT_READ_ULONG( rlen ) )
> ++ goto Exit2;
> ++
> ++ /* FT2 allocator takes signed long buffer length,
> ++ * too large fragment length causing overflow should be checked
> ++ */
> ++ if ( 0x7FFFFFFFUL < rlen )
> ++ {
> ++ error = FT_Err_Invalid_Offset;
> ++ goto Exit2;
> ++ }
> ++
> + if ( FT_READ_USHORT( flags ) )
> +- goto Exit;
> ++ goto Exit2;
> + FT_TRACE3(( "POST fragment[%d]: offsets=0x%08x, rlen=0x%08x, flags=0x%04x\n",
> + i, offsets[i], rlen, flags ));
> +
> ++ error = FT_Err_Array_Too_Large;
> + /* postpone the check of rlen longer than buffer until FT_Stream_Read() */
> + if ( ( flags >> 8 ) == 0 ) /* Comment, should not be loaded */
> ++ {
> ++ FT_TRACE3(( " Skip POST fragment #%d because it is a comment\n", i ));
> + continue;
> ++ }
> +
> + /* the flags are part of the resource, so rlen >= 2. */
> + /* but some fonts declare rlen = 0 for empty fragment */
> +@@ -1608,6 +1645,8 @@
> + len += rlen;
> + else
> + {
> ++ FT_TRACE3(( " Write POST fragment #%d header (4-byte) to buffer"
> ++ " 0x%p + 0x%08x\n", i, pfb_data, pfb_lenpos ));
> + if ( pfb_lenpos + 3 > pfb_len + 2 )
> + goto Exit2;
> + pfb_data[pfb_lenpos ] = (FT_Byte)( len );
> +@@ -1618,6 +1657,8 @@
> + if ( ( flags >> 8 ) == 5 ) /* End of font mark */
> + break;
> +
> ++ FT_TRACE3(( " Write POST fragment #%d header (6-byte) to buffer"
> ++ " 0x%p + 0x%08x\n", i, pfb_data, pfb_pos ));
> + if ( pfb_pos + 6 > pfb_len + 2 )
> + goto Exit2;
> + pfb_data[pfb_pos++] = 0x80;
> +@@ -1633,16 +1674,18 @@
> + pfb_data[pfb_pos++] = 0;
> + }
> +
> +- error = FT_Err_Cannot_Open_Resource;
> + if ( pfb_pos > pfb_len || pfb_pos + rlen > pfb_len )
> + goto Exit2;
> +
> ++ FT_TRACE3(( " Load POST fragment #%d (%d byte) to buffer"
> ++ " 0x%p + 0x%08x\n", i, rlen, pfb_data, pfb_pos ));
> + error = FT_Stream_Read( stream, (FT_Byte *)pfb_data + pfb_pos, rlen );
> + if ( error )
> + goto Exit2;
> + pfb_pos += rlen;
> + }
> +
> ++ error = FT_Err_Array_Too_Large;
> + if ( pfb_pos + 2 > pfb_len + 2 )
> + goto Exit2;
> + pfb_data[pfb_pos++] = 0x80;
> +@@ -1663,6 +1706,13 @@
> + aface );
> +
> + Exit2:
> ++ if ( error == FT_Err_Array_Too_Large )
> ++ FT_TRACE2(( " Abort due to too-short buffer to store"
> ++ " all POST fragments\n" ));
> ++ else if ( error == FT_Err_Invalid_Offset )
> ++ FT_TRACE2(( " Abort due to invalid offset in a POST fragment\n" ));
> ++ if ( error )
> ++ error = FT_Err_Cannot_Open_Resource;
> + FT_FREE( pfb_data );
> +
> + Exit:
> diff --git a/debian/patches-freetype/series b/debian/patches-freetype/series
> index 0c0aafc..fa6ce8c 100644
> --- a/debian/patches-freetype/series
> +++ b/debian/patches-freetype/series
> @@ -25,8 +25,8 @@ CVE-2014-9671-1.patch
> CVE-2014-9671-2.patch
> CVE-2014-9671-3.patch
> CVE-2014-9672.patch
> -CVE-2014-9673.patch
> CVE-2014-9675-1.patch
> CVE-2014-9675-2.patch
> savannah-bug-41309.patch
> savannah-bug-41590.patch
> +CVE-2014-9674+CVE-2014-9673.diff
Reply to: