Skip to content

bpo-30610: [Security] Python's libexpat vulnerable to CVE-2016-0718 #2021

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Misc/NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,9 @@ Extension Modules
Library
-------

- bpo-30610: [Security] Python's libexpat vulnerable to CVE-2016-0718.
Patch by Stéphane Wirtel.

- bpo-30418: On Windows, subprocess.Popen.communicate() now also ignore EINVAL
on stdin.write() if the child process is still running but closed the pipe.

Expand Down
3 changes: 0 additions & 3 deletions Modules/expat/amigaconfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,6 @@
/* Define to 1 if you have the `bcopy' function. */
#define HAVE_BCOPY 1

/* Define to 1 if you have the <check.h> header file. */
#undef HAVE_CHECK_H

/* Define to 1 if you have the `memmove' function. */
#define HAVE_MEMMOVE 1

Expand Down
17 changes: 8 additions & 9 deletions Modules/expat/expat.h
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,7 @@ XML_SetEntityDeclHandler(XML_Parser parser,
XML_EntityDeclHandler handler);

/* OBSOLETE -- OBSOLETE -- OBSOLETE
This handler has been superceded by the EntityDeclHandler above.
This handler has been superseded by the EntityDeclHandler above.
It is provided here for backward compatibility.

This is called for a declaration of an unparsed (NDATA) entity.
Expand Down Expand Up @@ -915,8 +915,6 @@ XMLPARSEAPI(int)
XML_SetHashSalt(XML_Parser parser,
unsigned long hash_salt);

#define XML_HAS_SET_HASH_SALT /* Python Only: Defined for pyexpat.c. */

/* If XML_Parse or XML_ParseBuffer have returned XML_STATUS_ERROR, then
XML_GetErrorCode returns information about the error.
*/
Expand Down Expand Up @@ -975,9 +973,12 @@ XML_FreeContentModel(XML_Parser parser, XML_Content *model);

/* Exposing the memory handling functions used in Expat */
XMLPARSEAPI(void *)
XML_ATTR_MALLOC
XML_ATTR_ALLOC_SIZE(2)
XML_MemMalloc(XML_Parser parser, size_t size);

XMLPARSEAPI(void *)
XML_ATTR_ALLOC_SIZE(3)
XML_MemRealloc(XML_Parser parser, void *ptr, size_t size);

XMLPARSEAPI(void)
Expand Down Expand Up @@ -1033,14 +1034,12 @@ XMLPARSEAPI(const XML_Feature *)
XML_GetFeatureList(void);


/* Expat follows the GNU/Linux convention of odd number minor version for
beta/development releases and even number minor version for stable
releases. Micro is bumped with each release, and set to 0 with each
change to major or minor version.
/* Expat follows the semantic versioning convention.
See http://semver.org.
*/
#define XML_MAJOR_VERSION 2
#define XML_MINOR_VERSION 1
#define XML_MICRO_VERSION 1
#define XML_MINOR_VERSION 2
#define XML_MICRO_VERSION 0

#ifdef __cplusplus
}
Expand Down
18 changes: 14 additions & 4 deletions Modules/expat/expat_external.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,6 @@

/* External API definitions */

/* Namespace external symbols to allow multiple libexpat version to
co-exist. */
#include "pyexpatns.h"

#if defined(_MSC_EXTENSIONS) && !defined(__BEOS__) && !defined(__CYGWIN__)
#define XML_USE_MSC_EXTENSIONS 1
#endif
Expand Down Expand Up @@ -69,12 +65,26 @@
#endif
#endif /* not defined XML_STATIC */

#if !defined(XMLIMPORT) && defined(__GNUC__) && (__GNUC__ >= 4)
#define XMLIMPORT __attribute__ ((visibility ("default")))
#endif

/* If we didn't define it above, define it away: */
#ifndef XMLIMPORT
#define XMLIMPORT
#endif

#if defined(__GNUC__) && (__GNUC__ > 2 || (__GNUC__ == 2 && __GNUC_MINOR__ >= 96))
#define XML_ATTR_MALLOC __attribute__((__malloc__))
#else
#define XML_ATTR_MALLOC
#endif

#if defined(__GNUC__) && ((__GNUC__ > 4) || (__GNUC__ == 4 && __GNUC_MINOR__ >= 3))
#define XML_ATTR_ALLOC_SIZE(x) __attribute__((__alloc_size__(x)))
#else
#define XML_ATTR_ALLOC_SIZE(x)
#endif

#define XMLPARSEAPI(type) XMLIMPORT type XMLCALL

Expand Down
22 changes: 22 additions & 0 deletions Modules/expat/internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -71,3 +71,25 @@
#define inline
#endif
#endif

#ifndef UNUSED_P
# ifdef __GNUC__
# define UNUSED_P(p) UNUSED_ ## p __attribute__((__unused__))
# else
# define UNUSED_P(p) UNUSED_ ## p
# endif
#endif


#ifdef __cplusplus
extern "C" {
#endif


void
align_limit_to_full_utf8_characters(const char * from, const char ** fromLimRef);


#ifdef __cplusplus
}
#endif
113 changes: 79 additions & 34 deletions Modules/expat/xmlparse.c
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,22 @@
See the file COPYING for copying permission.
*/

#include <stddef.h>
#include <string.h> /* memset(), memcpy() */
#include <assert.h>
#include <limits.h> /* UINT_MAX */

#ifdef WIN32
#define getpid GetCurrentProcessId
#else
#include <sys/time.h> /* gettimeofday() */
#include <sys/types.h> /* getpid() */
#include <unistd.h> /* getpid() */
#endif

#define XML_BUILDING_EXPAT 1

#ifdef COMPILED_FROM_DSP
#ifdef WIN32
#include "winconfig.h"
#elif defined(MACOS_CLASSIC)
#include "macconfig.h"
Expand All @@ -14,13 +27,7 @@
#include "watcomconfig.h"
#elif defined(HAVE_EXPAT_CONFIG_H)
#include <expat_config.h>
#endif /* ndef COMPILED_FROM_DSP */

#include <stddef.h>
#include <string.h> /* memset(), memcpy() */
#include <assert.h>
#include <limits.h> /* UINT_MAX */
#include <time.h> /* time() */
#endif /* ndef WIN32 */

#include "ascii.h"
#include "expat.h"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -691,19 +698,46 @@ 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 WIN32
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 * (unsigned long)2305843009213693951;
}
}

static XML_Bool /* only valid for root parser */
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
Expand Down Expand Up @@ -1695,15 +1729,15 @@ XML_GetBuffer(XML_Parser parser, int len)
if (len > bufferLim - bufferEnd) {
#ifdef XML_CONTEXT_BYTES
int keep;
#endif
int neededSize = len + (int)(bufferEnd - bufferPtr);
#endif /* defined XML_CONTEXT_BYTES */
/* Do not invoke signed arithmetic overflow: */
int neededSize = (int) ((unsigned)len + (unsigned)(bufferEnd - bufferPtr));
if (neededSize < 0) {
errorCode = XML_ERROR_NO_MEMORY;
return NULL;
}
#ifdef XML_CONTEXT_BYTES
keep = (int)(bufferPtr - buffer);

if (keep > XML_CONTEXT_BYTES)
keep = XML_CONTEXT_BYTES;
neededSize += keep;
Expand All @@ -1728,7 +1762,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);
} while (bufferSize < neededSize && bufferSize > 0);
if (bufferSize <= 0) {
errorCode = XML_ERROR_NO_MEMORY;
Expand Down Expand Up @@ -1855,7 +1890,7 @@ XML_Index XMLCALL
XML_GetCurrentByteIndex(XML_Parser parser)
{
if (eventPtr)
return parseEndByteIndex - (parseEndPtr - eventPtr);
return (XML_Index)(parseEndByteIndex - (parseEndPtr - eventPtr));
return -1;
}

Expand Down Expand Up @@ -2429,11 +2464,11 @@ doContent(XML_Parser parser,
for (;;) {
int bufSize;
int convLen;
XmlConvert(enc,
const enum XML_Convert_Result convert_res = XmlConvert(enc,
&fromPtr, rawNameEnd,
(ICHAR **)&toPtr, (ICHAR *)tag->bufEnd - 1);
convLen = (int)(toPtr - (XML_Char *)tag->buf);
if (fromPtr == rawNameEnd) {
if ((convert_res == XML_CONVERT_COMPLETED) || (convert_res == XML_CONVERT_INPUT_INCOMPLETE)) {
tag->name.strLen = convLen;
break;
}
Expand Down Expand Up @@ -2654,11 +2689,11 @@ doContent(XML_Parser parser,
if (MUST_CONVERT(enc, s)) {
for (;;) {
ICHAR *dataPtr = (ICHAR *)dataBuf;
XmlConvert(enc, &s, next, &dataPtr, (ICHAR *)dataBufEnd);
const enum XML_Convert_Result convert_res = XmlConvert(enc, &s, next, &dataPtr, (ICHAR *)dataBufEnd);
*eventEndPP = s;
charDataHandler(handlerArg, dataBuf,
(int)(dataPtr - (ICHAR *)dataBuf));
if (s == next)
if ((convert_res == XML_CONVERT_COMPLETED) || (convert_res == XML_CONVERT_INPUT_INCOMPLETE))
break;
*eventPP = s;
}
Expand Down Expand Up @@ -3264,11 +3299,11 @@ doCdataSection(XML_Parser parser,
if (MUST_CONVERT(enc, s)) {
for (;;) {
ICHAR *dataPtr = (ICHAR *)dataBuf;
XmlConvert(enc, &s, next, &dataPtr, (ICHAR *)dataBufEnd);
const enum XML_Convert_Result convert_res = XmlConvert(enc, &s, next, &dataPtr, (ICHAR *)dataBufEnd);
*eventEndPP = next;
charDataHandler(handlerArg, dataBuf,
(int)(dataPtr - (ICHAR *)dataBuf));
if (s == next)
if ((convert_res == XML_CONVERT_COMPLETED) || (convert_res == XML_CONVERT_INPUT_INCOMPLETE))
break;
*eventPP = s;
}
Expand Down Expand Up @@ -4927,9 +4962,9 @@ internalEntityProcessor(XML_Parser parser,

static enum XML_Error PTRCALL
errorProcessor(XML_Parser parser,
const char *s,
const char *end,
const char **nextPtr)
const char *UNUSED_P(s),
const char *UNUSED_P(end),
const char **UNUSED_P(nextPtr))
{
return errorCode;
}
Expand Down Expand Up @@ -5345,6 +5380,7 @@ reportDefault(XML_Parser parser, const ENCODING *enc,
const char *s, const char *end)
{
if (MUST_CONVERT(enc, s)) {
enum XML_Convert_Result convert_res;
const char **eventPP;
const char **eventEndPP;
if (enc == encoding) {
Expand All @@ -5357,11 +5393,11 @@ reportDefault(XML_Parser parser, const ENCODING *enc,
}
do {
ICHAR *dataPtr = (ICHAR *)dataBuf;
XmlConvert(enc, &s, end, &dataPtr, (ICHAR *)dataBufEnd);
convert_res = XmlConvert(enc, &s, end, &dataPtr, (ICHAR *)dataBufEnd);
*eventEndPP = s;
defaultHandler(handlerArg, dataBuf, (int)(dataPtr - (ICHAR *)dataBuf));
*eventPP = s;
} while (s != end);
} while ((convert_res != XML_CONVERT_COMPLETED) && (convert_res != XML_CONVERT_INPUT_INCOMPLETE));
}
else
defaultHandler(handlerArg, (XML_Char *)s, (int)((XML_Char *)end - (XML_Char *)s));
Expand Down Expand Up @@ -6166,8 +6202,8 @@ poolAppend(STRING_POOL *pool, const ENCODING *enc,
if (!pool->ptr && !poolGrow(pool))
return NULL;
for (;;) {
XmlConvert(enc, &ptr, end, (ICHAR **)&(pool->ptr), (ICHAR *)pool->end);
if (ptr == end)
const enum XML_Convert_Result convert_res = XmlConvert(enc, &ptr, end, (ICHAR **)&(pool->ptr), (ICHAR *)pool->end);
if ((convert_res == XML_CONVERT_COMPLETED) || (convert_res == XML_CONVERT_INPUT_INCOMPLETE))
break;
if (!poolGrow(pool))
return NULL;
Expand Down Expand Up @@ -6251,8 +6287,13 @@ poolGrow(STRING_POOL *pool)
}
}
if (pool->blocks && pool->start == pool->blocks->s) {
int blockSize = (int)(pool->end - pool->start)*2;
BLOCK *temp = (BLOCK *)
BLOCK *temp;
int blockSize = (int)((unsigned)(pool->end - pool->start)*2U);

if (blockSize < 0)
return XML_FALSE;

temp = (BLOCK *)
pool->mem->realloc_fcn(pool->blocks,
(offsetof(BLOCK, s)
+ blockSize * sizeof(XML_Char)));
Expand All @@ -6267,6 +6308,10 @@ poolGrow(STRING_POOL *pool)
else {
BLOCK *tem;
int blockSize = (int)(pool->end - pool->start);

if (blockSize < 0)
return XML_FALSE;

if (blockSize < INIT_BLOCK_SIZE)
blockSize = INIT_BLOCK_SIZE;
else
Expand Down
Loading