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