commit 22fe2da8e2bc0625d3c492f42d6b716adb36d5c2
Author: Tomas Korbar <tkorbar@redhat.com>
Date: Mon Feb 14 12:09:42 2022 +0100
CVE-2022-23852
diff --git a/lib/xmlparse.c b/lib/xmlparse.c
index 85ee0a8..4552680 100644
--- a/lib/xmlparse.c
+++ b/lib/xmlparse.c
@@ -161,6 +161,9 @@ typedef char ICHAR;
/* Round up n to be a multiple of sz, where sz is a power of 2. */
#define ROUND_UP(n, sz) (((n) + ((sz) - 1)) & ~((sz) - 1))
+/* Do safe (NULL-aware) pointer arithmetic */
+#define EXPAT_SAFE_PTR_DIFF(p, q) (((p) && (q)) ? ((p) - (q)) : 0)
+
/* Handle the case where memmove() doesn't exist. */
#ifndef HAVE_MEMMOVE
#ifdef HAVE_BCOPY
@@ -2026,39 +2029,54 @@ XML_GetBuffer(XML_Parser parser, int len)
default: ;
}
- if (len > parser->m_bufferLim - parser->m_bufferEnd) {
-#ifdef XML_CONTEXT_BYTES
+ if (len > EXPAT_SAFE_PTR_DIFF(parser->m_bufferLim, parser->m_bufferEnd)) {
int keep;
-#endif /* defined XML_CONTEXT_BYTES */
/* Do not invoke signed arithmetic overflow: */
- int neededSize = (int) ((unsigned)len + (unsigned)(parser->m_bufferEnd - parser->m_bufferPtr));
+ int neededSize = (int)((unsigned)len
+ + (unsigned)EXPAT_SAFE_PTR_DIFF(
+ parser->m_bufferEnd, parser->m_bufferPtr));
if (neededSize < 0) {
parser->m_errorCode = XML_ERROR_NO_MEMORY;
return NULL;
}
-#ifdef XML_CONTEXT_BYTES
- keep = (int)(parser->m_bufferPtr - parser->m_buffer);
+
+ keep = (int)EXPAT_SAFE_PTR_DIFF(parser->m_bufferPtr, parser->m_buffer);
if (keep > XML_CONTEXT_BYTES)
keep = XML_CONTEXT_BYTES;
+ /* Detect and prevent integer overflow */
+ if (keep > INT_MAX - neededSize) {
+ parser->m_errorCode = XML_ERROR_NO_MEMORY;
+ return NULL;
+ }
neededSize += keep;
-#endif /* defined XML_CONTEXT_BYTES */
- if (neededSize <= parser->m_bufferLim - parser->m_buffer) {
+ if (neededSize
+ <= EXPAT_SAFE_PTR_DIFF(parser->m_bufferLim, parser->m_buffer)) {
#ifdef XML_CONTEXT_BYTES
- if (keep < parser->m_bufferPtr - parser->m_buffer) {
- int offset = (int)(parser->m_bufferPtr - parser->m_buffer) - keep;
- memmove(parser->m_buffer, &parser->m_buffer[offset], parser->m_bufferEnd - parser->m_bufferPtr + keep);
+ if (keep < EXPAT_SAFE_PTR_DIFF(parser->m_bufferPtr, parser->m_buffer)) {
+ int offset
+ = (int)EXPAT_SAFE_PTR_DIFF(parser->m_bufferPtr, parser->m_buffer)
+ - keep;
+ /* The buffer pointers cannot be NULL here; we have at least some bytes
+ * in the buffer */
+ memmove(parser->m_buffer, &parser->m_buffer[offset],
+ parser->m_bufferEnd - parser->m_bufferPtr + keep);
parser->m_bufferEnd -= offset;
parser->m_bufferPtr -= offset;
}
#else
- memmove(parser->m_buffer, parser->m_bufferPtr, parser->m_bufferEnd - parser->m_bufferPtr);
- parser->m_bufferEnd = parser->m_buffer + (parser->m_bufferEnd - parser->m_bufferPtr);
- parser->m_bufferPtr = parser->m_buffer;
-#endif /* not defined XML_CONTEXT_BYTES */
- }
- else {
+ if (parser->m_buffer && parser->m_bufferPtr) {
+ memmove(parser->m_buffer, parser->m_bufferPtr,
+ EXPAT_SAFE_PTR_DIFF(parser->m_bufferEnd, parser->m_bufferPtr));
+ parser->m_bufferEnd
+ = parser->m_buffer
+ + EXPAT_SAFE_PTR_DIFF(parser->m_bufferEnd, parser->m_bufferPtr);
+ parser->m_bufferPtr = parser->m_buffer;
+ }
+#endif /* not defined XML_CONTEXT_BYTES */
+ } else {
char *newBuf;
- int bufferSize = (int)(parser->m_bufferLim - parser->m_bufferPtr);
+ int bufferSize
+ = (int)EXPAT_SAFE_PTR_DIFF(parser->m_bufferLim, parser->m_bufferPtr);
if (bufferSize == 0)
bufferSize = INIT_BUFFER_SIZE;
do {
@@ -2077,25 +2095,33 @@ XML_GetBuffer(XML_Parser parser, int len)
parser->m_bufferLim = newBuf + bufferSize;
#ifdef XML_CONTEXT_BYTES
if (parser->m_bufferPtr) {
- int keep = (int)(parser->m_bufferPtr - parser->m_buffer);
- if (keep > XML_CONTEXT_BYTES)
- keep = XML_CONTEXT_BYTES;
- memcpy(newBuf, &parser->m_bufferPtr[-keep], parser->m_bufferEnd - parser->m_bufferPtr + keep);
+ memcpy(newBuf, &parser->m_bufferPtr[-keep],
+ EXPAT_SAFE_PTR_DIFF(parser->m_bufferEnd, parser->m_bufferPtr)
+ + keep);
FREE(parser, parser->m_buffer);
parser->m_buffer = newBuf;
- parser->m_bufferEnd = parser->m_buffer + (parser->m_bufferEnd - parser->m_bufferPtr) + keep;
+ parser->m_bufferEnd
+ = parser->m_buffer
+ + EXPAT_SAFE_PTR_DIFF(parser->m_bufferEnd, parser->m_bufferPtr)
+ + keep;
parser->m_bufferPtr = parser->m_buffer + keep;
- }
- else {
- parser->m_bufferEnd = newBuf + (parser->m_bufferEnd - parser->m_bufferPtr);
+ } else {
+ /* This must be a brand new buffer with no data in it yet */
+ parser->m_bufferEnd = newBuf;
parser->m_bufferPtr = parser->m_buffer = newBuf;
}
#else
if (parser->m_bufferPtr) {
- memcpy(newBuf, parser->m_bufferPtr, parser->m_bufferEnd - parser->m_bufferPtr);
+ memcpy(newBuf, parser->m_bufferPtr,
+ EXPAT_SAFE_PTR_DIFF(parser->m_bufferEnd, parser->m_bufferPtr));
FREE(parser, parser->m_buffer);
+ parser->m_bufferEnd
+ = newBuf
+ + EXPAT_SAFE_PTR_DIFF(parser->m_bufferEnd, parser->m_bufferPtr);
+ } else {
+ /* This must be a brand new buffer with no data in it yet */
+ parser->m_bufferEnd = newBuf;
}
- parser->m_bufferEnd = newBuf + (parser->m_bufferEnd - parser->m_bufferPtr);
parser->m_bufferPtr = parser->m_buffer = newBuf;
#endif /* not defined XML_CONTEXT_BYTES */
}
diff --git a/tests/runtests.c b/tests/runtests.c
index e1f1ad1..ecc6f47 100644
--- a/tests/runtests.c
+++ b/tests/runtests.c
@@ -4116,6 +4116,31 @@ START_TEST(test_get_buffer_2)
}
END_TEST
+/* Test for signed integer overflow CVE-2022-23852 */
+#if defined(XML_CONTEXT_BYTES)
+START_TEST(test_get_buffer_3_overflow) {
+ XML_Parser parser = XML_ParserCreate(NULL);
+ assert(parser != NULL);
+
+ const char *const text = "\n";
+ const int expectedKeepValue = (int)strlen(text);
+
+ // After this call, variable "keep" in XML_GetBuffer will
+ // have value expectedKeepValue
+ if (XML_Parse(parser, text, (int)strlen(text), XML_FALSE /* isFinal */)
+ == XML_STATUS_ERROR)
+ xml_failure(parser);
+
+ assert(expectedKeepValue > 0);
+ if (XML_GetBuffer(parser, INT_MAX - expectedKeepValue + 1) != NULL)
+ fail("enlarging buffer not failed");
+
+ XML_ParserFree(parser);
+}
+END_TEST
+#endif // defined(XML_CONTEXT_BYTES)
+
+
/* Test position information macros */
START_TEST(test_byte_info_at_end)
{
@@ -12117,6 +12142,9 @@ make_suite(void)
tcase_add_test(tc_basic, test_empty_parse);
tcase_add_test(tc_basic, test_get_buffer_1);
tcase_add_test(tc_basic, test_get_buffer_2);
+#if defined(XML_CONTEXT_BYTES)
+ tcase_add_test(tc_basic, test_get_buffer_3_overflow);
+#endif
tcase_add_test(tc_basic, test_byte_info_at_end);
tcase_add_test(tc_basic, test_byte_info_at_error);
tcase_add_test(tc_basic, test_byte_info_at_cdata);