[CVE-2022-29824] Fix integer overflows in xmlBuf and xmlBuffer

In several places, the code handling string buffers didn't check for
integer overflow or used wrong types for buffer sizes. This could
result in out-of-bounds writes or other memory errors when working on
large, multi-gigabyte buffers.

Thanks to Felix Wilhelm for the report.

1 files changed, 33 insertions, 51 deletions

diff --git a/buf.c b/buf.c
index 9d95e801..2b1fd793 100644
--- a/buf.c
+++ b/buf.c
@@ -26,6 +26,10 @@
 #include <libxml/parserInternals.h> /* for XML_MAX_TEXT_LENGTH */
 #include "buf.h"
 
+#ifndef SIZE_MAX
+#define SIZE_MAX ((size_t) -1)
+#endif
+
 #define WITH_BUFFER_COMPAT
 
 /**
@@ -151,6 +155,8 @@ xmlBufPtr
 xmlBufCreateSize(size_t size) {
     xmlBufPtr ret;
 
+    if (size == SIZE_MAX)
+        return(NULL);
     ret = (xmlBufPtr) xmlMalloc(sizeof(xmlBuf));
     if (ret == NULL) {
 	xmlBufMemoryError(NULL, "creating buffer");
@@ -160,7 +166,7 @@ xmlBufCreateSize(size_t size) {
     ret->error = 0;
     ret->buffer = NULL;
     ret->alloc = xmlBufferAllocScheme;
-    ret->size = (size ? size+2 : 0);         /* +1 for ending null */
+    ret->size = (size ? size + 1 : 0);         /* +1 for ending null */
     UPDATE_COMPAT(ret);
     if (ret->size){
         ret->content = (xmlChar *) xmlMallocAtomic(ret->size * sizeof(xmlChar));
@@ -429,23 +435,17 @@ xmlBufGrowInternal(xmlBufPtr buf, size_t len) {
     CHECK_COMPAT(buf)
 
     if (buf->alloc == XML_BUFFER_ALLOC_IMMUTABLE) return(0);
-    if (buf->use + len < buf->size)
+    if (len < buf->size - buf->use)
         return(buf->size - buf->use);
+    if (len > SIZE_MAX - buf->use)
+        return(0);
 
-    /*
-     * Windows has a BIG problem on realloc timing, so we try to double
-     * the buffer size (if that's enough) (bug 146697)
-     * Apparently BSD too, and it's probably best for linux too
-     * On an embedded system this may be something to change
-     */
-#if 1
-    if (buf->size > (size_t) len)
-        size = buf->size * 2;
-    else
-        size = buf->use + len + 100;
-#else
-    size = buf->use + len + 100;
-#endif
+    if (buf->size > (size_t) len) {
+        size = buf->size > SIZE_MAX / 2 ? SIZE_MAX : buf->size * 2;
+    } else {
+        size = buf->use + len;
+        size = size > SIZE_MAX - 100 ? SIZE_MAX : size + 100;
+    }
 
     if (buf->alloc == XML_BUFFER_ALLOC_BOUNDED) {
         /*
@@ -731,7 +731,7 @@ xmlBufIsEmpty(const xmlBufPtr buf)
 int
 xmlBufResize(xmlBufPtr buf, size_t size)
 {
-    unsigned int newSize;
+    size_t newSize;
     xmlChar* rebuf = NULL;
     size_t start_buf;
 
@@ -759,9 +759,13 @@ xmlBufResize(xmlBufPtr buf, size_t size)
 	case XML_BUFFER_ALLOC_IO:
 	case XML_BUFFER_ALLOC_DOUBLEIT:
 	    /*take care of empty case*/
-	    newSize = (buf->size ? buf->size*2 : size + 10);
+            if (buf->size == 0) {
+                newSize = (size > SIZE_MAX - 10 ? SIZE_MAX : size + 10);
+            } else {
+                newSize = buf->size;
+            }
 	    while (size > newSize) {
-	        if (newSize > UINT_MAX / 2) {
+	        if (newSize > SIZE_MAX / 2) {
 	            xmlBufMemoryError(buf, "growing buffer");
 	            return 0;
 	        }
@@ -769,15 +773,15 @@ xmlBufResize(xmlBufPtr buf, size_t size)
 	    }
 	    break;
 	case XML_BUFFER_ALLOC_EXACT:
-	    newSize = size+10;
+            newSize = (size > SIZE_MAX - 10 ? SIZE_MAX : size + 10);
 	    break;
         case XML_BUFFER_ALLOC_HYBRID:
             if (buf->use < BASE_BUFFER_SIZE)
                 newSize = size;
             else {
-                newSize = buf->size * 2;
+                newSize = buf->size;
                 while (size > newSize) {
-                    if (newSize > UINT_MAX / 2) {
+                    if (newSize > SIZE_MAX / 2) {
                         xmlBufMemoryError(buf, "growing buffer");
                         return 0;
                     }
@@ -787,7 +791,7 @@ xmlBufResize(xmlBufPtr buf, size_t size)
             break;
 
 	default:
-	    newSize = size+10;
+            newSize = (size > SIZE_MAX - 10 ? SIZE_MAX : size + 10);
 	    break;
     }
 
@@ -853,7 +857,7 @@ xmlBufResize(xmlBufPtr buf, size_t size)
  */
 int
 xmlBufAdd(xmlBufPtr buf, const xmlChar *str, int len) {
-    unsigned int needSize;
+    size_t needSize;
 
     if ((str == NULL) || (buf == NULL) || (buf->error))
 	return -1;
@@ -875,8 +879,10 @@ xmlBufAdd(xmlBufPtr buf, const xmlChar *str, int len) {
     if (len < 0) return -1;
     if (len == 0) return 0;
 
-    needSize = buf->use + len + 2;
-    if (needSize > buf->size){
+    if ((size_t) len >= buf->size - buf->use) {
+        if ((size_t) len >= SIZE_MAX - buf->use)
+            return(-1);
+        needSize = buf->use + len + 1;
 	if (buf->alloc == XML_BUFFER_ALLOC_BOUNDED) {
 	    /*
 	     * Used to provide parsing limits
@@ -1012,31 +1018,7 @@ xmlBufCat(xmlBufPtr buf, const xmlChar *str) {
  */
 int
 xmlBufCCat(xmlBufPtr buf, const char *str) {
-    const char *cur;
-
-    if ((buf == NULL) || (buf->error))
-        return(-1);
-    CHECK_COMPAT(buf)
-    if (buf->alloc == XML_BUFFER_ALLOC_IMMUTABLE) return -1;
-    if (str == NULL) {
-#ifdef DEBUG_BUFFER
-        xmlGenericError(xmlGenericErrorContext,
-		"xmlBufCCat: str == NULL\n");
-#endif
-	return -1;
-    }
-    for (cur = str;*cur != 0;cur++) {
-        if (buf->use  + 10 >= buf->size) {
-            if (!xmlBufResize(buf, buf->use+10)){
-		xmlBufMemoryError(buf, "growing buffer");
-                return XML_ERR_NO_MEMORY;
-            }
-        }
-        buf->content[buf->use++] = *cur;
-    }
-    buf->content[buf->use] = 0;
-    UPDATE_COMPAT(buf)
-    return 0;
+    return xmlBufCat(buf, (const xmlChar *) str);
 }
 
 /**