184 lines
7.2 KiB
Diff
184 lines
7.2 KiB
Diff
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);
|