[PATCH 0/1] fontconfig: CVE-2016-5384

[PATCH 0/1] fontconfig: CVE-2016-5384

[Leo Famulari]

This patch uses a graft to apply the upstream fix to fontconfig for
CVE-2016-5384. I learned about the bug from a Debian security advisory:

https://security-tracker.debian.org/tracker/CVE-2016-5384
https://www.debian.org/security/2016/dsa-3644

Another potential option is to try grafting the latest version of
fontconfig, 2.12.1.

One way or another, ~2000 packages are depend on fontconfig.

Thoughts?

Leo Famulari (1):
  gnu: fontconfig: Fix CVE-2016-5384.

 gnu/local.mk                                       |   1 +
 gnu/packages/fontutils.scm                         |   8 +
 .../patches/fontconfig-CVE-2016-5384.patch         | 170 +++++++++++++++++++++
 3 files changed, 179 insertions(+)
 create mode 100644 gnu/packages/patches/fontconfig-CVE-2016-5384.patch

-- 
2.9.2

[PATCH 1/1] gnu: fontconfig: Fix CVE-2016-5384.

[Leo Famulari]

* gnu/packages/fontutils.scm (fontconfig)[replacement]: New field.
(fontconfig/fixed): New variable.
* gnu/packages/patches/fontconfig-CVE-2016-5384.patch: New file.
* gnu/local.mk (dist_patch_DATA): Add it.
---
 gnu/local.mk                                       |   1 +
 gnu/packages/fontutils.scm                         |   8 +
 .../patches/fontconfig-CVE-2016-5384.patch         | 170 +++++++++++++++++++++
 3 files changed, 179 insertions(+)
 create mode 100644 gnu/packages/patches/fontconfig-CVE-2016-5384.patch

diff --git a/gnu/local.mk b/gnu/local.mk
index c0671e8..7630b2d 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -505,6 +505,7 @@ dist_patch_DATA =						\
   %D%/packages/patches/flint-ldconfig.patch			\
   %D%/packages/patches/fltk-shared-lib-defines.patch		\
   %D%/packages/patches/fltk-xfont-on-demand.patch		\
+  %D%/packages/patches/fontconfig-CVE-2016-5384.patch		\
   %D%/packages/patches/fontforge-svg-modtime.patch		\
   %D%/packages/patches/fossil-test-fixes.patch			\
   %D%/packages/patches/freeimage-CVE-2015-0852.patch		\
diff --git a/gnu/packages/fontutils.scm b/gnu/packages/fontutils.scm
index 9ddbaec..4867164 100644
--- a/gnu/packages/fontutils.scm
+++ b/gnu/packages/fontutils.scm
@@ -223,6 +223,7 @@ fonts to/from the WOFF2 format.")
 (define-public fontconfig
   (package
    (name "fontconfig")
+   (replacement fontconfig/fixed)
    (version "2.11.94")
    (source (origin
             (method url-fetch)
@@ -275,6 +276,13 @@ high quality, anti-aliased and subpixel rendered text on a display.")
                        "See COPYING in the distribution."))
    (home-page "http://www.freedesktop.org/wiki/Software/fontconfig")))
 
+(define fontconfig/fixed
+  (package
+    (inherit fontconfig)
+    (source (origin
+              (inherit (package-source fontconfig))
+              (patches (search-patches "fontconfig-CVE-2016-5384.patch"))))))
+
 (define-public t1lib
   (package
    (name "t1lib")
diff --git a/gnu/packages/patches/fontconfig-CVE-2016-5384.patch b/gnu/packages/patches/fontconfig-CVE-2016-5384.patch
new file mode 100644
index 0000000..617d5af
--- /dev/null
+++ b/gnu/packages/patches/fontconfig-CVE-2016-5384.patch
@@ -0,0 +1,170 @@
+Fix CVE-2016-5384 (double-free resulting in arbitrary code execution):
+
+<https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2016-5384>
+
+Copied from upstream code repository:
+
+<https://cgit.freedesktop.org/fontconfig/commit/?id=7a4a5bd7897d216f0794ca9dbce0a4a5c9d14940>
+
+From 7a4a5bd7897d216f0794ca9dbce0a4a5c9d14940 Mon Sep 17 00:00:00 2001
+From: Tobias Stoeckmann <tobias@stoeckmann.org>
+Date: Sat, 25 Jun 2016 19:18:53 +0200
+Subject: Properly validate offsets in cache files.
+
+The cache files are insufficiently validated. Even though the magic
+number at the beginning of the file as well as time stamps are checked,
+it is not verified if contained offsets are in legal ranges or are
+even pointers.
+
+The lack of validation allows an attacker to trigger arbitrary free()
+calls, which in turn allows double free attacks and therefore arbitrary
+code execution. Due to the conversion from offsets into pointers through
+macros, this even allows to circumvent ASLR protections.
+
+This attack vector allows privilege escalation when used with setuid
+binaries like fbterm. A user can create ~/.fonts or any other
+system-defined user-private font directory, run fc-cache and adjust
+cache files in ~/.cache/fontconfig. The execution of setuid binaries will
+scan these files and therefore are prone to attacks.
+
+If it's not about code execution, an endless loop can be created by
+letting linked lists become circular linked lists.
+
+This patch verifies that:
+
+- The file is not larger than the maximum addressable space, which
+  basically only affects 32 bit systems. This allows out of boundary
+  access into unallocated memory.
+- Offsets are always positive or zero
+- Offsets do not point outside file boundaries
+- No pointers are allowed in cache files, every "pointer or offset"
+  field must be an offset or NULL
+- Iterating linked lists must not take longer than the amount of elements
+  specified. A violation of this rule can break a possible endless loop.
+
+If one or more of these points are violated, the cache is recreated.
+This is current behaviour.
+
+Even though this patch fixes many issues, the use of mmap() shall be
+forbidden in setuid binaries. It is impossible to guarantee with these
+checks that a malicious user does not change cache files after
+verification. This should be handled in a different patch.
+
+Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
+
+diff --git a/src/fccache.c b/src/fccache.c
+index 71e8f03..02ec301 100644
+--- a/src/fccache.c
++++ b/src/fccache.c
+@@ -27,6 +27,7 @@
+ #include <fcntl.h>
+ #include <dirent.h>
+ #include <string.h>
++#include <limits.h>
+ #include <sys/types.h>
+ #include <sys/stat.h>
+ #include <assert.h>
+@@ -587,6 +588,82 @@ FcCacheTimeValid (FcConfig *config, FcCache *cache, struct stat *dir_stat)
+     return cache->checksum == (int) dir_stat->st_mtime && fnano;
+ }
+ 
++static FcBool
++FcCacheOffsetsValid (FcCache *cache)
++{
++    char		*base = (char *)cache;
++    char		*end = base + cache->size;
++    intptr_t		*dirs;
++    FcFontSet		*fs;
++    int			 i, j;
++
++    if (cache->dir < 0 || cache->dir > cache->size - sizeof (intptr_t) ||
++        memchr (base + cache->dir, '\0', cache->size - cache->dir) == NULL)
++        return FcFalse;
++
++    if (cache->dirs < 0 || cache->dirs >= cache->size ||
++        cache->dirs_count < 0 ||
++        cache->dirs_count > (cache->size - cache->dirs) / sizeof (intptr_t))
++        return FcFalse;
++
++    dirs = FcCacheDirs (cache);
++    if (dirs)
++    {
++        for (i = 0; i < cache->dirs_count; i++)
++        {
++            FcChar8	*dir;
++
++            if (dirs[i] < 0 ||
++                dirs[i] > end - (char *) dirs - sizeof (intptr_t))
++                return FcFalse;
++
++            dir = FcOffsetToPtr (dirs, dirs[i], FcChar8);
++            if (memchr (dir, '\0', end - (char *) dir) == NULL)
++                return FcFalse;
++         }
++    }
++
++    if (cache->set < 0 || cache->set > cache->size - sizeof (FcFontSet))
++        return FcFalse;
++
++    fs = FcCacheSet (cache);
++    if (fs)
++    {
++        if (fs->nfont > (end - (char *) fs) / sizeof (FcPattern))
++            return FcFalse;
++
++        if (fs->fonts != 0 && !FcIsEncodedOffset(fs->fonts))
++            return FcFalse;
++
++        for (i = 0; i < fs->nfont; i++)
++        {
++            FcPattern		*font = FcFontSetFont (fs, i);
++            FcPatternElt	*e;
++            FcValueListPtr	 l;
++
++            if ((char *) font < base ||
++                (char *) font > end - sizeof (FcFontSet) ||
++                font->elts_offset < 0 ||
++                font->elts_offset > end - (char *) font ||
++                font->num > (end - (char *) font - font->elts_offset) / sizeof (FcPatternElt))
++                return FcFalse;
++
++
++            e = FcPatternElts(font);
++            if (e->values != 0 && !FcIsEncodedOffset(e->values))
++                return FcFalse;
++
++            for (j = font->num, l = FcPatternEltValues(e); j >= 0 && l; j--, l = FcValueListNext(l))
++                if (l->next != NULL && !FcIsEncodedOffset(l->next))
++                    break;
++            if (j < 0)
++                return FcFalse;
++        }
++    }
++
++    return FcTrue;
++}
++
+ /*
+  * Map a cache file into memory
+  */
+@@ -596,7 +673,8 @@ FcDirCacheMapFd (FcConfig *config, int fd, struct stat *fd_stat, struct stat *di
+     FcCache	*cache;
+     FcBool	allocated = FcFalse;
+ 
+-    if (fd_stat->st_size < (int) sizeof (FcCache))
++    if (fd_stat->st_size > INTPTR_MAX ||
++        fd_stat->st_size < (int) sizeof (FcCache))
+ 	return NULL;
+     cache = FcCacheFindByStat (fd_stat);
+     if (cache)
+@@ -652,6 +730,7 @@ FcDirCacheMapFd (FcConfig *config, int fd, struct stat *fd_stat, struct stat *di
+     if (cache->magic != FC_CACHE_MAGIC_MMAP ||
+ 	cache->version < FC_CACHE_VERSION_NUMBER ||
+ 	cache->size != (intptr_t) fd_stat->st_size ||
++        !FcCacheOffsetsValid (cache) ||
+ 	!FcCacheTimeValid (config, cache, dir_stat) ||
+ 	!FcCacheInsert (cache, fd_stat))
+     {
+-- 
+cgit v0.10.2
+
-- 
2.9.2

Re: [PATCH 0/1] fontconfig: CVE-2016-5384

[Mark H Weaver]

Leo Famulari <leo@famulari.name> writes:

> This patch uses a graft to apply the upstream fix to fontconfig for
> CVE-2016-5384. I learned about the bug from a Debian security advisory:
>
> https://security-tracker.debian.org/tracker/CVE-2016-5384
> https://www.debian.org/security/2016/dsa-3644

Looks good to me.  Please push.

   Thank you!
      Mark

Re: [PATCH 0/1] fontconfig: CVE-2016-5384

[Leo Famulari]

On Mon, Aug 08, 2016 at 07:17:50PM -0400, Mark H Weaver wrote:
> Leo Famulari <leo@famulari.name> writes:
> 
> > This patch uses a graft to apply the upstream fix to fontconfig for
> > CVE-2016-5384. I learned about the bug from a Debian security advisory:
> >
> > https://security-tracker.debian.org/tracker/CVE-2016-5384
> > https://www.debian.org/security/2016/dsa-3644
> 
> Looks good to me.  Please push.

Thanks for the review! Pushed as 6b5e654d