From mboxrd@z Thu Jan 1 00:00:00 1970 From: Leo Famulari Subject: Re: [PATCH 0/3] Expat and libxslt changes for core-updates Date: Thu, 9 Jun 2016 19:19:35 -0400 Message-ID: <20160609231935.GA14894@jasmine> References: <20160608101016.GA20565@debian-netbook> <20160609164317.GA5540@jasmine> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="HcAYCG3uE/tztfnV" Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:38066) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bB9Ew-0001A5-KJ for guix-devel@gnu.org; Thu, 09 Jun 2016 19:19:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bB9Er-0002pW-Vl for guix-devel@gnu.org; Thu, 09 Jun 2016 19:19:53 -0400 Received: from out2-smtp.messagingengine.com ([66.111.4.26]:33624) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bB9Ep-0002oy-It for guix-devel@gnu.org; Thu, 09 Jun 2016 19:19:49 -0400 Content-Disposition: inline In-Reply-To: <20160609164317.GA5540@jasmine> List-Id: "Development of GNU Guix and the GNU System distribution." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org Sender: "Guix-devel" To: Efraim Flashner Cc: guix-devel@gnu.org --HcAYCG3uE/tztfnV Content-Type: text/plain; charset=us-ascii Content-Disposition: inline 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. --HcAYCG3uE/tztfnV Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0001-gnu-expat-Fix-CVE-2012-6702-CVE-2016-0718-and-CVE-20.patch" >From a4a3a09b40c5f98b2c2a3d15458ab086ce867c3d Mon Sep 17 00:00:00 2001 From: Leo Famulari 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 +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 /* memset(), memcpy() */ + #include + #include /* UINT_MAX */ +-#include /* time() */ ++ ++#ifdef COMPILED_FROM_DSP ++#define getpid GetCurrentProcessId ++#else ++#include /* gettimeofday() */ ++#include /* getpid() */ ++#include /* 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 -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 --HcAYCG3uE/tztfnV--