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

From 6c283d83eccd940bcde15634ac8c7f100e3caefd Mon Sep 17 00:00:00 2001
From: Nick Wellnhofer <wellnhofer@aevum.de>
Date: Tue, 8 Mar 2022 20:10:02 +0100

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.

Change-Id: If3a2ec6b91d5ddcbea476ad674d85476a37d2c07
Signed-off-by: JinWang An <jinwang.an@samsung.com>

diff --git a/buf.c b/buf.c
index d46da36..9d61fed 100644 (file)
--- a/buf.c
+++ b/buf.c
@@ -30,6 +30,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
 
 /**
@@ -156,6 +160,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");
@@ -166,7 +172,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 */
     ret->compat_size = (int) ret->size;
     if (ret->size){
         ret->content = (xmlChar *) xmlMallocAtomic(ret->size * sizeof(xmlChar));
@@ -442,23 +448,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) {
         /*
@@ -744,7 +744,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;
 
@@ -772,9 +772,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;
                }
@@ -782,15 +786,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;
                     }
@@ -800,7 +804,7 @@ xmlBufResize(xmlBufPtr buf, size_t size)
             break;
 
        default:
-           newSize = size+10;
+            newSize = (size > SIZE_MAX - 10 ? SIZE_MAX : size + 10);
            break;
     }
 
@@ -866,7 +870,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;
@@ -888,8 +892,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
@@ -1025,31 +1031,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);
 }
 
 /**

diff --git a/tree.c b/tree.c
index 08b1a50..fba1e0e 100644 (file)
--- a/tree.c
+++ b/tree.c
@@ -7065,6 +7065,8 @@ xmlBufferPtr
 xmlBufferCreateSize(size_t size) {
     xmlBufferPtr ret;
 
+    if (size >= UINT_MAX)
+        return(NULL);
     ret = (xmlBufferPtr) xmlMalloc(sizeof(xmlBuffer));
     if (ret == NULL) {
        xmlTreeErrMemory("creating buffer");
@@ -7072,7 +7074,7 @@ xmlBufferCreateSize(size_t size) {
     }
     ret->use = 0;
     ret->alloc = xmlBufferAllocScheme;
-    ret->size = (size ? size+2 : 0);         /* +1 for ending null */
+    ret->size = (size ? size + 1 : 0);         /* +1 for ending null */
     if (ret->size){
         ret->content = (xmlChar *) xmlMallocAtomic(ret->size * sizeof(xmlChar));
         if (ret->content == NULL) {
@@ -7132,6 +7134,8 @@ xmlBufferCreateStatic(void *mem, size_t size) {
 
     if ((mem == NULL) || (size == 0))
         return(NULL);
+    if (size > UINT_MAX)
+        return(NULL);
 
     ret = (xmlBufferPtr) xmlMalloc(sizeof(xmlBuffer));
     if (ret == NULL) {
@@ -7279,28 +7283,23 @@ xmlBufferShrink(xmlBufferPtr buf, unsigned int len) {
  */
 int
 xmlBufferGrow(xmlBufferPtr buf, unsigned int len) {
-    int size;
+    unsigned int size;
     xmlChar *newbuf;
 
     if (buf == NULL) return(-1);
 
     if (buf->alloc == XML_BUFFER_ALLOC_IMMUTABLE) return(0);
-    if (len + buf->use < buf->size) return(0);
+    if (len < buf->size - buf->use)
+        return(0);
+    if (len > UINT_MAX - buf->use)
+        return(-1);
 
-    /*
-     * 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 > 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 > UINT_MAX / 2 ? UINT_MAX : buf->size * 2;
+    } else {
+        size = buf->use + len;
+        size = size > UINT_MAX - 100 ? UINT_MAX : size + 100;
+    }
 
     if ((buf->alloc == XML_BUFFER_ALLOC_IO) && (buf->contentIO != NULL)) {
         size_t start_buf = buf->content - buf->contentIO;
@@ -7422,7 +7421,10 @@ xmlBufferResize(xmlBufferPtr buf, unsigned int 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 > UINT_MAX - 10 ? UINT_MAX : size + 10);
+            else
+                newSize = buf->size;
            while (size > newSize) {
                if (newSize > UINT_MAX / 2) {
                    xmlTreeErrMemory("growing buffer");
@@ -7432,7 +7434,7 @@ xmlBufferResize(xmlBufferPtr buf, unsigned int size)
            }
            break;
        case XML_BUFFER_ALLOC_EXACT:
-           newSize = size+10;
+           newSize = (size > UINT_MAX - 10 ? UINT_MAX : size + 10);;
            break;
         case XML_BUFFER_ALLOC_HYBRID:
             if (buf->use < BASE_BUFFER_SIZE)
@@ -7450,7 +7452,7 @@ xmlBufferResize(xmlBufferPtr buf, unsigned int size)
             break;
 
        default:
-           newSize = size+10;
+           newSize = (size > UINT_MAX - 10 ? UINT_MAX : size + 10);;
            break;
     }
 
@@ -7536,8 +7538,10 @@ xmlBufferAdd(xmlBufferPtr 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 ((unsigned) len >= buf->size - buf->use) {
+        if ((unsigned) len >= UINT_MAX - buf->use)
+            return XML_ERR_NO_MEMORY;
+        needSize = buf->use + len + 1;
         if (!xmlBufferResize(buf, needSize)){
            xmlTreeErrMemory("growing buffer");
             return XML_ERR_NO_MEMORY;
@@ -7650,29 +7654,7 @@ xmlBufferCat(xmlBufferPtr buf, const xmlChar *str) {
  */
 int
 xmlBufferCCat(xmlBufferPtr buf, const char *str) {
-    const char *cur;
-
-    if (buf == NULL)
-        return(-1);
-    if (buf->alloc == XML_BUFFER_ALLOC_IMMUTABLE) return -1;
-    if (str == NULL) {
-#ifdef DEBUG_BUFFER
-        xmlGenericError(xmlGenericErrorContext,
-               "xmlBufferCCat: str == NULL\n");
-#endif
-       return -1;
-    }
-    for (cur = str;*cur != 0;cur++) {
-        if (buf->use  + 10 >= buf->size) {
-            if (!xmlBufferResize(buf, buf->use+10)){
-               xmlTreeErrMemory("growing buffer");
-                return XML_ERR_NO_MEMORY;
-            }
-        }
-        buf->content[buf->use++] = *cur;
-    }
-    buf->content[buf->use] = 0;
-    return 0;
+    return xmlBufferCat(buf, (const xmlChar *) str);
 }
 
 /**