| 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 |
| --- |
| expat/CMakeLists.txt | 3 +++ |
| expat/lib/xmlparse.c | 48 +++++++++++++++++++++++++++++++++++++++++------- |
| 2 files changed, 44 insertions(+), 7 deletions(-) |
| |
| diff --git a/expat/CMakeLists.txt b/expat/CMakeLists.txt |
| index 353627e..524d514 100755 |
| --- a/expat/CMakeLists.txt |
| +++ b/expat/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/expat/lib/xmlparse.c b/expat/lib/xmlparse.c |
| index e308c79..c5f942f 100644 |
| --- a/expat/lib/xmlparse.c |
| +++ b/expat/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 |
| |