rpms/xmlrpc-c
6
0
Fork 0
Code Issues Pull Requests Releases Activity
c8
xmlrpc-c/SOURCES/0009-Restrict-XML-Entity-Expansion-Depth-in-libexpat-CVE-.patch
eabdullin c7d69ca874 Import from CS git
2025-04-22 07:43:01 +00:00

474 lines
15 KiB
Diff
Raw Permalink Blame History

From 6d8259c43c7d675987d96bf990b6b57402d27f28 Mon Sep 17 00:00:00 2001
From: root <root@vm-10-0-185-207.hosted.upshift.rdu2.redhat.com>
Date: Wed, 19 Mar 2025 12:52:04 -0400
Subject: [PATCH 09/10] Restrict XML Entity Expansion Depth in libexpat
CVE-2024-8176
The embedded libexpat library is vulnerable to a stack overflow
due to uncontrolled recursion when processing deeply nested XML
entities. This can cause the application to crash, resulting in
a denial of service (DoS) or potentially leading to memory
corruption, depending on the user's environment and how the
library is used. The issue is triggered by supplying a specially
crafted XML document designed to create a long chain of recursive
entities.
libexpat addressed this upstream in
https://github.com/libexpat/libexpat/pull/973
but the embedded copy within xmlrpc-c is so old there is no chance
of applying this without rebasing it. Instead a recursion counter
is added to the parser to limit the depth.
Resolves: RHEL-57536
---
lib/expat/xmlparse/xmlparse.c | 40 +++++++++++++---
test/Makefile | 30 ++++++++++--
test/overflow_attr_test.c | 88 +++++++++++++++++++++++++++++++++++
test/overflow_entity_test.c | 86 ++++++++++++++++++++++++++++++++++
4 files changed, 234 insertions(+), 10 deletions(-)
create mode 100644 test/overflow_attr_test.c
create mode 100644 test/overflow_entity_test.c
diff --git a/lib/expat/xmlparse/xmlparse.c b/lib/expat/xmlparse/xmlparse.c
index c440ac7..467910a 100644
--- a/lib/expat/xmlparse/xmlparse.c
+++ b/lib/expat/xmlparse/xmlparse.c
@@ -29,6 +29,8 @@ See the file copying.txt for copying permission.
#include "xmldef.h"
#include "xmlparse.h"
+#define MAX_RECURSE 28
+
static const char *
extractXmlSample(const char * const start,
const char * const end,
@@ -312,6 +314,7 @@ typedef struct {
enum XML_ParamEntityParsing m_paramEntityParsing;
XML_Parser m_parentParser;
unsigned long m_hash_secret_salt;
+ int m_recurse_lock;
} Parser;
#define userData (((Parser *)parser)->m_userData)
@@ -388,6 +391,7 @@ typedef struct {
#define namespaceSeparator (((Parser *)parser)->m_namespaceSeparator)
#define parentParser (((Parser *)parser)->m_parentParser)
#define hash_secret_salt (((Parser *)parser)->m_hash_secret_salt)
+#define recurse_lock (((Parser *)parser)->m_recurse_lock)
@@ -515,7 +519,7 @@ poolGrow(STRING_POOL * const poolP,
size_t const newSize =
offsetof(BLOCK, s) + blockSize * sizeof(XML_Char);
- BLOCK * const newBlocksP = malloc(newSize);
+ BLOCK * const newBlocksP = calloc(offsetof(BLOCK, s) + blockSize, sizeof(XML_Char));
if (newBlocksP) {
newBlocksP->size = blockSize;
@@ -1143,6 +1147,7 @@ const XML_Char *getContext(XML_Parser parser)
{
HASH_TABLE_ITER iter;
int needSep = 0;
+ Parser * const parserP = (Parser *) parser;
if (dtd.defaultPrefix.binding) {
int i;
@@ -1191,7 +1196,7 @@ const XML_Char *getContext(XML_Parser parser)
ENTITY *e = (ENTITY *)hashTableIterNext(&iter);
if (!e)
break;
- if (!e->open)
+ if (!e->open || parserP->m_recurse_lock > MAX_RECURSE)
continue;
if (needSep && !poolAppendChar(&tempPool, CONTEXT_SEP))
return 0;
@@ -1210,6 +1215,7 @@ static
int setContext(XML_Parser parser, const XML_Char *context)
{
const XML_Char *s = context;
+ Parser * const parserP = (Parser *) parser;
while (*context != XML_T('\0')) {
if (*s == CONTEXT_SEP || *s == XML_T('\0')) {
@@ -1218,8 +1224,9 @@ int setContext(XML_Parser parser, const XML_Char *context)
return 0;
e = (ENTITY *)lookup(parser, &dtd.generalEntities, poolStart(&tempPool),
0);
- if (e)
+ if (e) {
e->open = 1;
+ }
if (*s != XML_T('\0'))
s++;
context = s;
@@ -1860,7 +1867,7 @@ appendAttributeValue(XML_Parser const xmlParserP,
return XML_ERROR_UNDEFINED_ENTITY;
}
}
- else if (entity->open) {
+ else if (entity->open || parser->m_recurse_lock > MAX_RECURSE) {
if (enc == parser->m_encoding)
eventPtr = ptr;
return XML_ERROR_RECURSIVE_ENTITY_REF;
@@ -1879,10 +1886,12 @@ appendAttributeValue(XML_Parser const xmlParserP,
enum XML_Error result;
const XML_Char *textEnd = entity->textPtr + entity->textLen;
entity->open = 1;
+ parser->m_recurse_lock++;
result = appendAttributeValue(xmlParserP, internalEncoding,
isCdata, (char *)entity->textPtr,
(char *)textEnd, pool);
entity->open = 0;
+ parser->m_recurse_lock--;
if (result)
return result;
}
@@ -1952,7 +1961,7 @@ storeEntityValue(XML_Parser const xmlParserP,
eventPtr = entityTextPtr;
return XML_ERROR_UNDEFINED_ENTITY;
}
- if (entity->open) {
+ if (entity->open || parser->m_recurse_lock > MAX_RECURSE) {
if (enc == parser->m_encoding)
eventPtr = entityTextPtr;
return XML_ERROR_RECURSIVE_ENTITY_REF;
@@ -2641,8 +2650,13 @@ doInternalEntityRef(XML_Parser const xmlParserP,
*errorP = NULL;
} else {
OPEN_INTERNAL_ENTITY openEntity;
+ if (entityP->open || parserP->m_recurse_lock > MAX_RECURSE) {
+ *errorCodeP = XML_ERROR_RECURSIVE_ENTITY_REF;
+ return;
+ }
entityP->open = 1; /* recursion control */
+ parserP->m_recurse_lock++;
openEntity.next = parserP->m_openInternalEntities;
@@ -2659,6 +2673,7 @@ doInternalEntityRef(XML_Parser const xmlParserP,
0, errorCodeP, errorP);
entityP->open = 0; /* recursion control */
+ parserP->m_recurse_lock--;
parserP->m_openInternalEntities = openEntity.next;
}
}
@@ -2681,8 +2696,14 @@ doExternalEntityRef(XML_Parser const xmlParserP,
if (parserP->m_externalEntityRefHandler) {
const XML_Char * contextP;
+ if (entityP->open || parserP->m_recurse_lock > MAX_RECURSE) {
+ *errorCodeP = XML_ERROR_RECURSIVE_ENTITY_REF;
+ return;
+ }
entityP->open = 1;
+ parserP->m_recurse_lock++;
contextP = getContext(xmlParserP);
+ parserP->m_recurse_lock--;
entityP->open = 0;
if (!contextP)
@@ -2753,7 +2774,7 @@ doEntityRef(XML_Parser const xmlParserP,
*errorCodeP = XML_ERROR_NONE;
}
} else {
- if (entityP->open)
+ if (entityP->open || parserP->m_recurse_lock > MAX_RECURSE)
*errorCodeP = XML_ERROR_RECURSIVE_ENTITY_REF;
else if (entityP->notation)
*errorCodeP = XML_ERROR_BINARY_ENTITY_REF;
@@ -4059,7 +4080,7 @@ doProlog(XML_Parser const xmlParserP,
*errorCodeP = XML_ERROR_UNDEFINED_ENTITY;
return;
}
- if (entity->open) {
+ if (entity->open || parser->m_recurse_lock > MAX_RECURSE) {
*errorCodeP = XML_ERROR_RECURSIVE_ENTITY_REF;
return;
}
@@ -4076,16 +4097,19 @@ doProlog(XML_Parser const xmlParserP,
if (externalEntityRefHandler) {
dtd.complete = 0;
entity->open = 1;
+ parser->m_recurse_lock++;
if (!externalEntityRefHandler(externalEntityRefHandlerArg,
0,
entity->base,
entity->systemId,
entity->publicId)) {
entity->open = 0;
+ parser->m_recurse_lock--;
*errorCodeP = XML_ERROR_EXTERNAL_ENTITY_HANDLING;
return;
}
entity->open = 0;
+ parser->m_recurse_lock--;
if (dtd.complete)
break;
}
@@ -4199,6 +4223,7 @@ processInternalParamEntity(XML_Parser const parser,
const char *s, *end, *next;
int tok;
OPEN_INTERNAL_ENTITY openEntity;
+ Parser * const parserP = (Parser *) parser;
entity->open = 1;
openEntity.next = openInternalEntities;
@@ -4292,6 +4317,7 @@ xmlrpc_XML_ParserCreate(const XML_Char * const encodingName) {
parser->m_paramEntityParsing = XML_PARAM_ENTITY_PARSING_NEVER;
parser->m_hash_secret_salt = 0;
parser->m_ns = 0;
+ parser->m_recurse_lock = 0;
poolInit(&parser->m_tempPool);
poolInit(&parser->m_temp2Pool);
parser->m_protocolEncodingName =
diff --git a/test/Makefile b/test/Makefile
index 1242910..59c698c 100644
--- a/test/Makefile
+++ b/test/Makefile
@@ -16,9 +16,9 @@ LDADD_CGI_SERVER = \
default: all
-INCLUDES = -I$(BLDDIR) -Isrcdir/include -Isrcdir/lib/util/include \
+INCLUDES = -I$(BLDDIR) -I$(SRCDIR)/include -I$(SRCDIR)/lib/util/include \
-PROGS = test cgitest1
+PROGS = test cgitest1 overflow_attr_test overflow_entity_test
all: $(PROGS) $(SUBDIRS:%=%/all)
@@ -86,6 +86,28 @@ OBJS = $(TEST_OBJS) cgitest1.o
$(OBJS):%.o:%.c
$(CC) -c $(INCLUDES) $(CFLAGS_ALL) $<
+OVERFLOW_ATTR_OBJS = overflow_attr_test.o
+
+overflow_attr_test: $(OVERFLOW_ATTR_OBJS) $(LIBXMLRPC_SERVER_A) \
+ $(LIBXMLRPC_A) $(LIBXMLRPC_UTIL_A) $(LIBXMLRPC_XML)
+ $(CCLD) -o $@ $(LDFLAGS_ALL) \
+ $(OVERFLOW_ATTR_OBJS) $(LDADD_CLIENT) $(LDADD_ABYSS_SERVER) $(CASPRINTF)
+OBJS = overflow_attr_test.o
+
+$(OBJS):%.o:%.c
+ $(CC) -c $(INCLUDES) -I$(SRCDIR)/lib/expat/xmlparse/ $(CFLAGS_ALL) $<
+
+OVERFLOW_ENTITY_OBJS = overflow_entity_test.o
+
+overflow_entity_test: $(OVERFLOW_ENTITY_OBJS) $(LIBXMLRPC_SERVER_A) \
+ $(LIBXMLRPC_A) $(LIBXMLRPC_UTIL_A) $(LIBXMLRPC_XML)
+ $(CCLD) -o $@ $(LDFLAGS_ALL) \
+ $(OVERFLOW_ENTITY_OBJS) $(LDADD_CLIENT) $(LDADD_ABYSS_SERVER) $(CASPRINTF)
+OBJS = overflow_entity_test.o
+
+$(OBJS):%.o:%.c
+ $(CC) -c $(INCLUDES) -I$(SRCDIR)/lib/expat/xmlparse/ $(CFLAGS_ALL) $<
+
# Note the difference between 'check' and 'runtests'. 'check' means to check
# our own correctness. 'runtests' means to run the tests that check our
# parent's correctness
@@ -94,8 +116,10 @@ $(OBJS):%.o:%.c
check:
.PHONY: runtests_local
-runtests_local: test cgitest1
+runtests_local: test cgitest1 overflow_attr_test overflow_entity_test
./test
+ ./overflow_attr_test
+ ./overflow_entity_test
.PHONY: runtests
runtests: runtests_local cpp/runtests benchmark/runtests
diff --git a/test/overflow_attr_test.c b/test/overflow_attr_test.c
new file mode 100644
index 0000000..eb06556
--- /dev/null
+++ b/test/overflow_attr_test.c
@@ -0,0 +1,88 @@
+/* gcc -g -std=c11 -D_POSIX_C_SOURCE=200809L -I lib/expat/xmlparse -lxmlrpc_xmlparse -lxmlrpc_xmltok overflow_attr_test.c -o overflow_attr_test */
+
+#include <err.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <sys/stat.h>
+#include "xmlparse.h"
+#include <string.h>
+
+#define XML_STATUS_OK 1
+#define XML_STATUS_ERROR 0
+
+typedef unsigned char XML_Bool;
+#define XML_TRUE ((XML_Bool)1)
+#define XML_FALSE ((XML_Bool)0)
+
+#define SIZE_PER_LINE 50
+
+static int
+doParse(int N_LINES) {
+ XML_Parser parser;
+ char tmp[SIZE_PER_LINE];
+ int ret = 0;
+ int rval = 0;
+
+ if ((parser = xmlrpc_XML_ParserCreate(NULL)) == NULL)
+ errx(1, "XML_ParserCreate");
+
+ char *const text = (char *)malloc((N_LINES + 4) * SIZE_PER_LINE);
+ if (text == NULL) {
+ printf("malloc failed");
+ return 1;
+ }
+
+ // Create the XML
+ snprintf(text, SIZE_PER_LINE,
+ "<!DOCTYPE foo [\n"
+ " <!ENTITY s0 'deepText'>\n");
+
+ for (size_t i = 1; i < N_LINES; ++i) {
+ snprintf(tmp, SIZE_PER_LINE, " <!ENTITY s%lu '&s%lu;'>\n",
+ (long unsigned)i, (long unsigned)(i - 1));
+ strncat(text, tmp, SIZE_PER_LINE);
+ }
+
+ snprintf(tmp, SIZE_PER_LINE, "]> <foo name='&s%lu;'>mainText</foo>\n",
+ (long unsigned)(N_LINES - 1));
+ strncat(text, tmp, SIZE_PER_LINE);
+
+ ret = xmlrpc_XML_Parse(parser, text, strlen(text), XML_TRUE);
+ if (ret == XML_STATUS_ERROR) {
+ int err = xmlrpc_XML_GetErrorCode(parser);
+ if (err == XML_ERROR_RECURSIVE_ENTITY_REF) {
+ if (N_LINES <= 28) {
+ printf("%d Unexpected FAIL (%s)\n", N_LINES, xmlrpc_XML_ErrorString(err));
+ } else {
+ printf("%d expected failure OK (%s)\n", N_LINES, xmlrpc_XML_ErrorString(err));
+ }
+ } else {
+ printf("%d Unexpected FAIL (%s)\n", N_LINES, xmlrpc_XML_ErrorString(err));
+ rval = 1;
+ }
+ } else {
+ printf("%d OK\n", N_LINES);
+ }
+ free(text);
+ xmlrpc_XML_ParserFree(parser);
+ return rval;
+}
+
+int main(int argc, char *argv[]) {
+ int status = 0;
+
+ for (int i = 2; i <= 28; i++ )
+ status += doParse(i);
+
+ status += doParse(29);
+ status += doParse(30);
+ status += doParse(100);
+ status += doParse(1000);
+ status += doParse(10000);
+ status += doParse(60000);
+
+ if (status == 0)
+ printf("PASSED\n");
+ return (status != 0);
+}
diff --git a/test/overflow_entity_test.c b/test/overflow_entity_test.c
new file mode 100644
index 0000000..d0d1290
--- /dev/null
+++ b/test/overflow_entity_test.c
@@ -0,0 +1,86 @@
+/* gcc -g -std=c11 -D_POSIX_C_SOURCE=200809L -I lib/expat/xmlparse -lxmlrpc_xmlparse -lxmlrpc_xmltok overflow_entity_test.c -o overflow_entity_test */
+
+#include <err.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <sys/stat.h>
+#include "xmlparse.h"
+#include <string.h>
+
+#define XML_STATUS_OK 1
+#define XML_STATUS_ERROR 0
+
+typedef unsigned char XML_Bool;
+#define XML_TRUE ((XML_Bool)1)
+#define XML_FALSE ((XML_Bool)0)
+
+#define SIZE_PER_LINE 50
+
+static int
+doParse(int N_LINES) {
+ XML_Parser parser;
+ char tmp[SIZE_PER_LINE];
+ int ret = 0;
+ int rval = 0;
+
+ if ((parser = xmlrpc_XML_ParserCreate(NULL)) == NULL)
+ errx(1, "XML_ParserCreate");
+
+ char *const text = (char *)malloc((N_LINES + 4) * SIZE_PER_LINE);
+ if (text == NULL) {
+ printf("malloc failed");
+ return 1;
+ }
+
+ // Create the XML
+ snprintf(text, SIZE_PER_LINE,
+ "<!DOCTYPE foo [\n"
+ " <!ENTITY s0 'deepText'>\n");
+
+ for (size_t i = 1; i < N_LINES; ++i) {
+ snprintf(tmp, SIZE_PER_LINE, " <!ENTITY s%lu '&s%lu;'>\n",
+ (long unsigned)i, (long unsigned)(i - 1));
+ strncat(text, tmp, SIZE_PER_LINE);
+ }
+
+ snprintf(tmp, SIZE_PER_LINE, "]> <foo>&s%lu;</foo>\n",
+ (long unsigned)(N_LINES - 1));
+ strncat(text, tmp, SIZE_PER_LINE);
+
+ ret = xmlrpc_XML_Parse(parser, text, strlen(text), XML_TRUE);
+ if (ret == XML_STATUS_ERROR){
+ int err = xmlrpc_XML_GetErrorCode(parser);
+ if (err == XML_ERROR_RECURSIVE_ENTITY_REF) {
+ if (N_LINES <= 28) {
+ printf("%d Unexpected FAIL (%s)\n", N_LINES, xmlrpc_XML_ErrorString(err));
+ rval = 1;
+ } else {
+ printf("%d expected failure OK (%s)\n", N_LINES, xmlrpc_XML_ErrorString(err));
+ }
+ }
+ } else {
+ printf("%d OK\n", N_LINES);
+ }
+ free(text);
+ xmlrpc_XML_ParserFree(parser);
+ return rval;
+}
+
+int main(int argc, char *argv[]) {
+ int status = 0;
+
+ for (int i = 2; i <= 28; i++ )
+ status += doParse(i);
+
+ status += doParse(29);
+ status += doParse(30);
+ status += doParse(100);
+ status += doParse(1000);
+ status += doParse(10000);
+ status += doParse(60000);
+
+ if (status == 0)
+ printf("PASSED\n");
+ return (status != 0);
+}
--
2.43.5
Reference in New Issue View Git Blame Copy Permalink