Attachment 171224 Details for Bug 210155 – Upstream patch for issue

[patch] Upstream patch for issue

cve-2012-6702-plus-cve-2016-5300-v1.patch (text/plain), 4.43 KB, created by Jason Unovitch on 2016-06-09 03:26:48 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jason Unovitch

Created: 2016-06-09 03:26:48 UTC

Size: 4.43 KB

patch

obsolete

>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
>

Actions: View | Diff

Attachments on bug 210155: 171224