Skip to content

Commit

Permalink
[CVE-2022-29824] Fix integer overflows in xmlBuf and xmlBuffer
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@nwellnhof
nwellnhof committed May 2, 2022
1 parent f687f3e commit 2554a24
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 97 deletions.
86 changes: 34 additions & 52 deletions buf.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

/**
Expand Down Expand Up @@ -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");
Expand All @@ -166,8 +172,8 @@ xmlBufCreateSize(size_t size) {
ret->error = 0;
ret->buffer = NULL;
ret->alloc = xmlBufferAllocScheme;
ret->size = (size ? size+2 : 0); /* +1 for ending null */
ret->compat_size = (int) ret->size;
ret->size = (size ? size + 1 : 0); /* +1 for ending null */
ret->compat_size = (ret->size > INT_MAX ? INT_MAX : ret->size);
if (ret->size){
ret->content = (xmlChar *) xmlMallocAtomic(ret->size * sizeof(xmlChar));
if (ret->content == NULL) {
Expand Down Expand Up @@ -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) {
/*
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -772,25 +772,29 @@ 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;
}
newSize *= 2;
}
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;
}
Expand All @@ -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;
}

Expand Down Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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);
}

/**
Expand Down
72 changes: 27 additions & 45 deletions tree.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7104,14 +7104,16 @@ xmlBufferPtr
xmlBufferCreateSize(size_t size) {
xmlBufferPtr ret;

if (size >= UINT_MAX)
return(NULL);
ret = (xmlBufferPtr) xmlMalloc(sizeof(xmlBuffer));
if (ret == NULL) {
xmlTreeErrMemory("creating buffer");
return(NULL);
}
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) {
Expand Down Expand Up @@ -7171,6 +7173,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) {
Expand Down Expand Up @@ -7318,28 +7322,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;
Expand Down Expand Up @@ -7466,7 +7465,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 : 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");
Expand All @@ -7476,7 +7478,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)
Expand All @@ -7494,7 +7496,7 @@ xmlBufferResize(xmlBufferPtr buf, unsigned int size)
break;

default:
newSize = size+10;
newSize = (size > UINT_MAX - 10 ? UINT_MAX : size + 10);;
break;
}

Expand Down Expand Up @@ -7580,8 +7582,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;
Expand Down Expand Up @@ -7694,29 +7698,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);
}

/**
Expand Down

0 comments on commit 2554a24

Please sign in to comment.