Re: [PATCH 0/3] Expat and libxslt changes for core-updates

[Leo Famulari <leo@famulari.name>]

[-- Attachment #1: Type: text/plain, Size: 1200 bytes --]

On Thu, Jun 09, 2016 at 12:43:17PM -0400, Leo Famulari wrote:
> On Wed, Jun 08, 2016 at 01:10:16PM +0300, Efraim Flashner wrote:
> > FWIW debian's expat-2.1.1(-3) still has the cve-2015-1283 applied.
> 
> I looked at the expat Git repo and the original fix for CVE-2015-1283
> was part of 2.1.1. The improvement to the fix must be backported. I will
> take the upstream commit that applies the improvement.

We adopt Debian's patch for CVE-2012-6702 and CVE-2016-5300 (already
sent for review for the master branch).

We also adapt the CVE-2015-1283 "re-fix" patch to apply to upstream's
fix for CVE-2015-1283. The Debian "re-fix" patch had some context
(comments) that did not exist in the upstream 2.1.1 release.

And as before, we patch for CVE-2016-0718.

It's not possible for me test this on core-updates (too much to build).
On master, I made a new expat-2.1.1 package that inherited from expat
and built that with the patches.

The merge will probably be messy...

Off-topic: A regular package and a grafted package on master, and an
updated version of the package on core-updates... this is getting very
complicated and we should try our best to avoid such tangled situations
in the future.

[-- Attachment #2: 0001-gnu-expat-Fix-CVE-2012-6702-CVE-2016-0718-and-CVE-20.patch --]
[-- Type: text/x-diff, Size: 9682 bytes --]

From a4a3a09b40c5f98b2c2a3d15458ab086ce867c3d Mon Sep 17 00:00:00 2001
From: Leo Famulari <leo@famulari.name>
Date: Tue, 7 Jun 2016 20:26:41 -0400
Subject: [v2 1/2] gnu: expat: Fix CVE-2012-6702, CVE-2016-0718, and
 CVE-2016-5300.

* gnu/packages/patches/expat-CVE-2012-6702-and-CVE-2016-5300.patch: New file.
* gnu/local.mk (dist_patch_DATA): Add it.
* gnu/packages/patches/expat-CVE-2015-1283-refix.patch: Adapt to upstream
changes.
* gnu/packages/xml.scm (expat)[source]: Use patches.
---
 gnu/local.mk                                       |   1 +
 .../expat-CVE-2012-6702-and-CVE-2016-5300.patch    | 142 +++++++++++++++++++++
 .../patches/expat-CVE-2015-1283-refix.patch        |  27 ++--
 gnu/packages/xml.scm                               |   4 +
 4 files changed, 159 insertions(+), 15 deletions(-)
 create mode 100644 gnu/packages/patches/expat-CVE-2012-6702-and-CVE-2016-5300.patch

diff --git a/gnu/local.mk b/gnu/local.mk
index ef7b4df..4ace667 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -480,6 +480,7 @@ dist_patch_DATA =						\
   %D%/packages/patches/emacs-source-date-epoch.patch		\
   %D%/packages/patches/eudev-rules-directory.patch		\
   %D%/packages/patches/evilwm-lost-focus-bug.patch		\
+  %D%/packages/patches/expat-CVE-2012-6702-and-CVE-2016-5300.patch	\
   %D%/packages/patches/expat-CVE-2015-1283-refix.patch		\
   %D%/packages/patches/expat-CVE-2016-0718.patch		\
   %D%/packages/patches/fastcap-mulGlobal.patch			\
diff --git a/gnu/packages/patches/expat-CVE-2012-6702-and-CVE-2016-5300.patch b/gnu/packages/patches/expat-CVE-2012-6702-and-CVE-2016-5300.patch
new file mode 100644
index 0000000..edc43f8
--- /dev/null
+++ b/gnu/packages/patches/expat-CVE-2012-6702-and-CVE-2016-5300.patch
@@ -0,0 +1,142 @@
+Fix CVE-2012-6702 and CVE-2016-5300.
+
+https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2012-6702
+https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2016-5300
+
+Patch copied from:
+https://sources.debian.net/src/expat/2.1.0-6%2Bdeb8u3/debian/patches/cve-2012-6702-plus-cve-2016-5300-v1.patch/
+
+From cb31522769d11a375078a073cba94e7176cb48a4 Mon Sep 17 00:00:00 2001
+From: Sebastian Pipping <sebastian@pipping.org>
+Date: Wed, 16 Mar 2016 15:30:12 +0100
+Subject: [PATCH] Resolve call to srand, use more entropy (patch version 1.0)
+
+Squashed backport against vanilla Expat 2.1.1, addressing:
+* CVE-2012-6702 -- unanticipated internal calls to srand
+* CVE-2016-5300 -- use of too little entropy
+
+Since commit e3e81a6d9f0885ea02d3979151c358f314bf3d6d
+(released with Expat 2.1.0) Expat called srand by itself
+from inside generate_hash_secret_salt for an instance
+of XML_Parser if XML_SetHashSalt was either (a) not called
+for that instance or if (b) salt 0 was passed to XML_SetHashSalt
+prior to parsing.  That call to srand passed (rather litle)
+entropy extracted from the current time as a seed for srand.
+
+That call to srand (1) broke repeatability for code calling
+srand with a non-random seed prior to parsing with Expat,
+and (2) resulted in a rather small set of hashing salts in
+Expat in total.
+
+For a short- to mid-term fix, the new approach avoids calling
+srand altogether, extracts more entropy out of the clock and
+other sources, too.
+
+For a long term fix, we may want to read sizeof(long) bytes
+from a source like getrandom(..) on Linux, and from similar
+sources on other supported architectures.
+
+https://bugzilla.redhat.com/show_bug.cgi?id=1197087
+---
+ CMakeLists.txt |  3 +++
+ lib/xmlparse.c | 48 +++++++++++++++++++++++++++++++++++++++++-------
+ 2 files changed, 44 insertions(+), 7 deletions(-)
+
+diff --git a/CMakeLists.txt b/CMakeLists.txt
+index 353627e..524d514 100755
+--- a/CMakeLists.txt
++++ b/CMakeLists.txt
+@@ -41,6 +41,9 @@ include_directories(${CMAKE_BINARY_DIR} ${CMAKE_SOURCE_DIR}/lib)
+ if(MSVC)
+     add_definitions(-D_CRT_SECURE_NO_WARNINGS -wd4996)
+ endif(MSVC)
++if(WIN32)
++    add_definitions(-DCOMPILED_FROM_DSP)
++endif(WIN32)
+ 
+ set(expat_SRCS
+     lib/xmlparse.c
+diff --git a/lib/xmlparse.c b/lib/xmlparse.c
+index e308c79..c5f942f 100644
+--- a/lib/xmlparse.c
++++ b/lib/xmlparse.c
+@@ -6,7 +6,14 @@
+ #include <string.h>                     /* memset(), memcpy() */
+ #include <assert.h>
+ #include <limits.h>                     /* UINT_MAX */
+-#include <time.h>                       /* time() */
++
++#ifdef COMPILED_FROM_DSP
++#define getpid GetCurrentProcessId
++#else
++#include <sys/time.h>                   /* gettimeofday() */
++#include <sys/types.h>                  /* getpid() */
++#include <unistd.h>                     /* getpid() */
++#endif
+ 
+ #define XML_BUILDING_EXPAT 1
+ 
+@@ -432,7 +439,7 @@ static ELEMENT_TYPE *
+ getElementType(XML_Parser parser, const ENCODING *enc,
+                const char *ptr, const char *end);
+ 
+-static unsigned long generate_hash_secret_salt(void);
++static unsigned long generate_hash_secret_salt(XML_Parser parser);
+ static XML_Bool startParsing(XML_Parser parser);
+ 
+ static XML_Parser
+@@ -691,11 +698,38 @@ static const XML_Char implicitContext[] = {
+ };
+ 
+ static unsigned long
+-generate_hash_secret_salt(void)
++gather_time_entropy(void)
+ {
+-  unsigned int seed = time(NULL) % UINT_MAX;
+-  srand(seed);
+-  return rand();
++#ifdef COMPILED_FROM_DSP
++  FILETIME ft;
++  GetSystemTimeAsFileTime(&ft); /* never fails */
++  return ft.dwHighDateTime ^ ft.dwLowDateTime;
++#else
++  struct timeval tv;
++  int gettimeofday_res;
++
++  gettimeofday_res = gettimeofday(&tv, NULL);
++  assert (gettimeofday_res == 0);
++
++  /* Microseconds time is <20 bits entropy */
++  return tv.tv_usec;
++#endif
++}
++
++static unsigned long
++generate_hash_secret_salt(XML_Parser parser)
++{
++  /* Process ID is 0 bits entropy if attacker has local access
++   * XML_Parser address is few bits of entropy if attacker has local access */
++  const unsigned long entropy =
++      gather_time_entropy() ^ getpid() ^ (unsigned long)parser;
++
++  /* Factors are 2^31-1 and 2^61-1 (Mersenne primes M31 and M61) */
++  if (sizeof(unsigned long) == 4) {
++    return entropy * 2147483647;
++  } else {
++    return entropy * 2305843009213693951;
++  }
+ }
+ 
+ static XML_Bool  /* only valid for root parser */
+@@ -703,7 +737,7 @@ startParsing(XML_Parser parser)
+ {
+     /* hash functions must be initialized before setContext() is called */
+     if (hash_secret_salt == 0)
+-      hash_secret_salt = generate_hash_secret_salt();
++      hash_secret_salt = generate_hash_secret_salt(parser);
+     if (ns) {
+       /* implicit context only set for root parser, since child
+          parsers (i.e. external entity parsers) will inherit it
+-- 
+2.8.2
+
diff --git a/gnu/packages/patches/expat-CVE-2015-1283-refix.patch b/gnu/packages/patches/expat-CVE-2015-1283-refix.patch
index af5e3bc..fc8d629 100644
--- a/gnu/packages/patches/expat-CVE-2015-1283-refix.patch
+++ b/gnu/packages/patches/expat-CVE-2015-1283-refix.patch
@@ -1,42 +1,39 @@
-Update previous fix for CVE-2015-1283 to not rely on undefined behavior.
+Follow-up upstream fix for CVE-2015-1283 to not rely on undefined
+behavior.
 
-Copied from Debian, as found in Debian package version 2.1.0-6+deb8u2.
+Adapted from a patch from Debian (found in Debian package version
+2.1.0-6+deb8u2) to apply to upstream code:
 
 https://sources.debian.net/src/expat/2.1.0-6%2Bdeb8u2/debian/patches/CVE-2015-1283-refix.patch/
 
-From 29a11774d8ebbafe8418b4a5ffb4cc1160b194a1 Mon Sep 17 00:00:00 2001
-From: Pascal Cuoq <cuoq@trust-in-soft.com>
-Date: Sun, 15 May 2016 09:05:46 +0200
-Subject: [PATCH] Avoid relying on undefined behavior in CVE-2015-1283 fix.
-
 ---
- expat/lib/xmlparse.c | 6 ++++--
+ lib/xmlparse.c | 6 ++++--
  1 file changed, 4 insertions(+), 2 deletions(-)
 
 diff --git a/lib/xmlparse.c b/lib/xmlparse.c
-index 13e080d..cdb12ef 100644
+index 0f6f4cd..5c70c17 100644
 --- a/lib/xmlparse.c
 +++ b/lib/xmlparse.c
-@@ -1695,7 +1695,8 @@ XML_GetBuffer(XML_Parser parser, int len
+@@ -1727,7 +1727,8 @@ XML_GetBuffer(XML_Parser parser, int len)
    }
  
    if (len > bufferLim - bufferEnd) {
 -    int neededSize = len + (int)(bufferEnd - bufferPtr);
 +    /* Do not invoke signed arithmetic overflow: */
 +    int neededSize = (int) ((unsigned)len + (unsigned)(bufferEnd - bufferPtr));
- /* BEGIN MOZILLA CHANGE (sanity check neededSize) */
      if (neededSize < 0) {
        errorCode = XML_ERROR_NO_MEMORY;
-@@ -1729,7 +1730,8 @@ XML_GetBuffer(XML_Parser parser, int len
+       return NULL;
+@@ -1759,7 +1760,8 @@ XML_GetBuffer(XML_Parser parser, int len)
        if (bufferSize == 0)
          bufferSize = INIT_BUFFER_SIZE;
        do {
 -        bufferSize *= 2;
 +        /* Do not invoke signed arithmetic overflow: */
 +        bufferSize = (int) (2U * (unsigned) bufferSize);
- /* BEGIN MOZILLA CHANGE (prevent infinite loop on overflow) */
        } while (bufferSize < neededSize && bufferSize > 0);
- /* END MOZILLA CHANGE */
+       if (bufferSize <= 0) {
+         errorCode = XML_ERROR_NO_MEMORY;
 -- 
-2.8.2
+2.8.3
 
diff --git a/gnu/packages/xml.scm b/gnu/packages/xml.scm
index a860f98..f3bb7d8 100644
--- a/gnu/packages/xml.scm
+++ b/gnu/packages/xml.scm
@@ -51,6 +51,10 @@
              (method url-fetch)
              (uri (string-append "mirror://sourceforge/expat/expat/"
                                  version "/expat-" version ".tar.bz2"))
+             (patches (search-patches
+                        "expat-CVE-2012-6702-and-CVE-2016-5300.patch"
+                        "expat-CVE-2015-1283-refix.patch"
+                        "expat-CVE-2016-0718.patch"))
              (sha256
               (base32
                "0ryyjgvy7jq0qb7a9mhc1giy3bzn56aiwrs8dpydqngplbjq9xdg"))))
-- 
2.8.3