commit 22fe2da8e2bc0625d3c492f42d6b716adb36d5c2 Author: Tomas Korbar 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);