From 6acb0a47372a9079cc6ff70c384f015a47f2c34a 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 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 adds some additional entropy from the process ID, 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/lib/xmlparse.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/expat/lib/xmlparse.c b/expat/lib/xmlparse.c index 2d0bd3b1..41299dae 100644 --- a/expat/lib/xmlparse.c +++ b/expat/lib/xmlparse.c @@ -6,7 +6,9 @@ #include /* memset(), memcpy() */ #include #include /* UINT_MAX */ -#include /* time() */ +#include /* gettimeofday() */ +#include /* getpid() */ +#include /* getpid() */ #define XML_BUILDING_EXPAT 1 @@ -693,9 +695,16 @@ static const XML_Char implicitContext[] = { static unsigned long generate_hash_secret_salt(void) { - unsigned int seed = time(NULL) % UINT_MAX; - srand(seed); - return rand(); + struct timeval tv; + int gettimeofday_res; + + gettimeofday_res = gettimeofday(&tv, NULL); + assert (gettimeofday_res == 0); + + /* Microseconds time is <20 bits entropy + * Process ID is 0 bits entropy if attacker has local access + * Factor is 2^61-1 (Mersenne prime M61) */ + return (tv.tv_usec ^ getpid()) * 2305843009213693951; } static XML_Bool /* only valid for root parser */ -- 2.40.0