GNU bug report logs - #29797
[PATCH] gnu: libxslt: Fix CVE-2017-5029 and re-apply the fix for CVE-2016-4738.

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 29797 in the body.
You can then email your comments to 29797 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Leo Famulari <leo <at> famulari.name>
To: guix-patches <at> gnu.org
Subject: [PATCH] gnu: libxslt: Fix CVE-2017-5029 and re-apply the fix for
 CVE-2016-4738.
Date: Thu, 21 Dec 2017 02:19:17 -0500

This is a followup to commit 2663c38826cd6c2ef0c5119f8072fac8e89b2e9b.

* gnu/packages/xml.scm (libxslt)[replacement]: New field.
(libxslt/fixed): New variable.
* gnu/packages/patches/libxslt-CVE-2017-5029.patch: New file.
* gnu/local.mk (dist_patch_DATA): Add it.
---
 gnu/local.mk                                     |  1 +
 gnu/packages/patches/libxslt-CVE-2017-5029.patch | 82 ++++++++++++++++++++++++
 gnu/packages/xml.scm                             | 13 ++++
 3 files changed, 96 insertions(+)
 create mode 100644 gnu/packages/patches/libxslt-CVE-2017-5029.patch

diff --git a/gnu/local.mk b/gnu/local.mk
index 8ffcc5800..f619d1363 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -849,6 +849,7 @@ dist_patch_DATA =						\
   %D%/packages/patches/libxml2-CVE-2017-9049+CVE-2017-9050.patch	\
   %D%/packages/patches/libxslt-generated-ids.patch		\
   %D%/packages/patches/libxslt-CVE-2016-4738.patch		\
+  %D%/packages/patches/libxslt-CVE-2017-5029.patch		\
   %D%/packages/patches/libxt-guix-search-paths.patch		\
   %D%/packages/patches/lierolibre-check-unaligned-access.patch	\
   %D%/packages/patches/lierolibre-is-free-software.patch	\
diff --git a/gnu/packages/patches/libxslt-CVE-2017-5029.patch b/gnu/packages/patches/libxslt-CVE-2017-5029.patch
new file mode 100644
index 000000000..cd86928b2
--- /dev/null
+++ b/gnu/packages/patches/libxslt-CVE-2017-5029.patch
@@ -0,0 +1,82 @@
+Fix CVE-2017-5029:
+
+https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-5029
+
+Patch copied from upstream source repository:
+
+https://git.gnome.org/browse/libxslt/commit/?id=08ab2774b870de1c7b5a48693df75e8154addae5
+
+From 08ab2774b870de1c7b5a48693df75e8154addae5 Mon Sep 17 00:00:00 2001
+From: Nick Wellnhofer <wellnhofer <at> aevum.de>
+Date: Thu, 12 Jan 2017 15:39:52 +0100
+Subject: [PATCH] Check for integer overflow in xsltAddTextString
+
+Limit buffer size in xsltAddTextString to INT_MAX. The issue can be
+exploited to trigger an out of bounds write on 64-bit systems.
+
+Originally reported to Chromium:
+
+https://crbug.com/676623
+---
+ libxslt/transform.c     | 25 ++++++++++++++++++++++---
+ libxslt/xsltInternals.h |  4 ++--
+ 2 files changed, 24 insertions(+), 5 deletions(-)
+
+diff --git a/libxslt/transform.c b/libxslt/transform.c
+index 519133fc..02bff34a 100644
+--- a/libxslt/transform.c
++++ b/libxslt/transform.c
+@@ -813,13 +813,32 @@ xsltAddTextString(xsltTransformContextPtr ctxt, xmlNodePtr target,
+         return(target);
+ 
+     if (ctxt->lasttext == target->content) {
++        int minSize;
+ 
+-	if (ctxt->lasttuse + len >= ctxt->lasttsize) {
++        /* Check for integer overflow accounting for NUL terminator. */
++        if (len >= INT_MAX - ctxt->lasttuse) {
++            xsltTransformError(ctxt, NULL, target,
++                "xsltCopyText: text allocation failed\n");
++            return(NULL);
++        }
++        minSize = ctxt->lasttuse + len + 1;
++
++        if (ctxt->lasttsize < minSize) {
+ 	    xmlChar *newbuf;
+ 	    int size;
++            int extra;
++
++            /* Double buffer size but increase by at least 100 bytes. */
++            extra = minSize < 100 ? 100 : minSize;
++
++            /* Check for integer overflow. */
++            if (extra > INT_MAX - ctxt->lasttsize) {
++                size = INT_MAX;
++            }
++            else {
++                size = ctxt->lasttsize + extra;
++            }
+ 
+-	    size = ctxt->lasttsize + len + 100;
+-	    size *= 2;
+ 	    newbuf = (xmlChar *) xmlRealloc(target->content,size);
+ 	    if (newbuf == NULL) {
+ 		xsltTransformError(ctxt, NULL, target,
+diff --git a/libxslt/xsltInternals.h b/libxslt/xsltInternals.h
+index 060b1783..5ad17719 100644
+--- a/libxslt/xsltInternals.h
++++ b/libxslt/xsltInternals.h
+@@ -1754,8 +1754,8 @@ struct _xsltTransformContext {
+      * Speed optimization when coalescing text nodes
+      */
+     const xmlChar  *lasttext;		/* last text node content */
+-    unsigned int    lasttsize;		/* last text node size */
+-    unsigned int    lasttuse;		/* last text node use */
++    int             lasttsize;		/* last text node size */
++    int             lasttuse;		/* last text node use */
+     /*
+      * Per Context Debugging
+      */
+-- 
+2.15.1
+
diff --git a/gnu/packages/xml.scm b/gnu/packages/xml.scm
index 4f75de344..9cf9e1411 100644
--- a/gnu/packages/xml.scm
+++ b/gnu/packages/xml.scm
@@ -188,6 +188,7 @@ project (but it is usable outside of the Gnome platform).")
 (define-public libxslt
   (package
     (name "libxslt")
+    (replacement libxslt/fixed)
     (version "1.1.29")
     (source (origin
              (method url-fetch)
@@ -197,6 +198,9 @@ project (but it is usable outside of the Gnome platform).")
              (sha256
               (base32
                "1klh81xbm9ppzgqk339097i39b7fnpmlj8lzn8bpczl3aww6x5xm"))
+             ;; XXX Oops, a redefinition of the patches field, which means the
+             ;; patch for CVE-2016-4738 is not used. Fixed in the definition of
+             ;; libxslt-fixed below.
              (patches (search-patches "libxslt-generated-ids.patch"))))
     (build-system gnu-build-system)
     (home-page "http://xmlsoft.org/XSLT/index.html")
@@ -210,6 +214,15 @@ project (but it is usable outside of the Gnome platform).")
 based on libxml for XML parsing, tree manipulation and XPath support.")
     (license license:x11)))
 
+(define libxslt/fixed
+  (package
+    (inherit libxslt)
+    (source (origin
+              (inherit (package-source libxslt))
+              (patches (search-patches "libxslt-CVE-2016-4738.patch"
+                                       "libxslt-CVE-2017-5029.patch"
+                                       "libxslt-generated-ids.patch"))))))
+
 (define-public perl-graph-readwrite
   (package
     (name "perl-graph-readwrite")
-- 
2.15.1

Message #8 received at 29797 <at> debbugs.gnu.org (full text, mbox):

From: ludo <at> gnu.org (Ludovic Courtès)
To: Leo Famulari <leo <at> famulari.name>
Cc: 29797 <at> debbugs.gnu.org
Subject: Re: [bug#29797] [PATCH] gnu: libxslt: Fix CVE-2017-5029 and re-apply
 the fix for CVE-2016-4738.
Date: Thu, 21 Dec 2017 11:15:46 +0100

Leo Famulari <leo <at> famulari.name> skribis:

> This is a followup to commit 2663c38826cd6c2ef0c5119f8072fac8e89b2e9b.
>
> * gnu/packages/xml.scm (libxslt)[replacement]: New field.
> (libxslt/fixed): New variable.
> * gnu/packages/patches/libxslt-CVE-2017-5029.patch: New file.
> * gnu/local.mk (dist_patch_DATA): Add it.

[...]

> --- a/gnu/packages/xml.scm
> +++ b/gnu/packages/xml.scm
> @@ -188,6 +188,7 @@ project (but it is usable outside of the Gnome platform).")
>  (define-public libxslt
>    (package
>      (name "libxslt")
> +    (replacement libxslt/fixed)
>      (version "1.1.29")
>      (source (origin
>               (method url-fetch)
> @@ -197,6 +198,9 @@ project (but it is usable outside of the Gnome platform).")
>               (sha256
>                (base32
>                 "1klh81xbm9ppzgqk339097i39b7fnpmlj8lzn8bpczl3aww6x5xm"))
> +             ;; XXX Oops, a redefinition of the patches field, which means the
> +             ;; patch for CVE-2016-4738 is not used. Fixed in the definition of
> +             ;; libxslt-fixed below.
>               (patches (search-patches "libxslt-generated-ids.patch"))))

Oops, indeed!  You can remove the unused ‘patches’ line while you’re at it.

>      (build-system gnu-build-system)
>      (home-page "http://xmlsoft.org/XSLT/index.html")
> @@ -210,6 +214,15 @@ project (but it is usable outside of the Gnome platform).")
>  based on libxml for XML parsing, tree manipulation and XPath support.")
>      (license license:x11)))
>  
> +(define libxslt/fixed
> +  (package
> +    (inherit libxslt)
> +    (source (origin
> +              (inherit (package-source libxslt))
> +              (patches (search-patches "libxslt-CVE-2016-4738.patch"
> +                                       "libxslt-CVE-2017-5029.patch"
> +                                       "libxslt-generated-ids.patch"))))))

LGTM, thanks!

Ludo’.

Message #13 received at 29797-done <at> debbugs.gnu.org (full text, mbox):

From: Leo Famulari <leo <at> famulari.name>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 29797-done <at> debbugs.gnu.org
Subject: Re: [bug#29797] [PATCH] gnu: libxslt: Fix CVE-2017-5029 and re-apply
 the fix for CVE-2016-4738.
Date: Thu, 21 Dec 2017 12:30:28 -0500

[Message part 1 (text/plain, inline)]

On Thu, Dec 21, 2017 at 11:15:46AM +0100, Ludovic Courtès wrote:
> Leo Famulari <leo <at> famulari.name> skribis:
> > +             ;; XXX Oops, a redefinition of the patches field, which means the
> > +             ;; patch for CVE-2016-4738 is not used. Fixed in the definition of
> > +             ;; libxslt-fixed below.
> >               (patches (search-patches "libxslt-generated-ids.patch"))))
> 
> Oops, indeed!  You can remove the unused ‘patches’ line while you’re at it.

I commented it out and pushed as
0c9c9526bb3fb665997b3b054f8b57ffdb559043.

[signature.asc (application/pgp-signature, inline)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.