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);