Subject: bug#29797: closed (Re: [bug#29797] [PATCH] gnu: libxslt: Fix CVE-2017-5029 and re-apply the fix for CVE-2016-4738.)

From: [email-masked] (GNU bug Tracking System)
Date: Thu, 21 Dec 2017 17:31:03 +0000

Your bug report

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

which was filed against the guix-patches package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
29797: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=29797
GNU Bug Tracking System
Contact [email-masked] with problems

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

On Thu, Dec 21, 2017 at 11:15:46AM +0100, Ludovic Courtès wrote:
> Leo Famulari <[email-masked]> 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.

Subject: [PATCH] gnu: libxslt: Fix CVE-2017-5029 and re-apply the fix for CVE-2016-4738.
From: Leo Famulari <[email-masked]>
To: [email-masked]
Date: Thu, 21 Dec 2017 02:19:17 -0500
List-Id: <debbugs-submit.debbugs.gnu.org>

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 <[email-masked]>
+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