CVE-2019-17178: Unchecked Return Value (CWE-252)
value: HuffmanTree_makeFromFrequencies in lodepng.c in LodePNG through 2019-09-28, as used in WinPR in FreeRDP and other products, has a memory leak because a supplied realloc pointer (i.e., the first argument to realloc) is also used for a realloc return value.
CWE description: The product does not check the return value from a method or function, which can prevent it from detecting unexpected states and conditions.
CVSS v3 base score: 7.5
Published: 2019-10-04T17:15Z
Repository: https://github.com/FreeRDP/FreeRDP
Language: C
Files changed: client/X11/generate_argument_docbook.c, libfreerdp/codec/region.c, winpr/libwinpr/utils/lodepng/lodepng.c
Commit date: 2019-10-04 16:19:23 +0200
Commit message: Fixed #5645: realloc return handling

Diff: diff --git a/client/X11/generate_argument_docbook.c b/client/X11/generate_argument_docbook.c
index b700539e27..1a3ebf563b 100644
--- a/client/X11/generate_argument_docbook.c
+++ b/client/X11/generate_argument_docbook.c
@@ -7,10 +7,11 @@
 
 #define TAG FREERDP_TAG("generate_argument_docbook")
 LPSTR tr_esc_str(LPCSTR arg, bool format)
 {
 	LPSTR tmp = NULL;
+	LPSTR tmp2 = NULL;
 	size_t cs = 0, x, ds, len;
 	size_t s;
 
 	if (NULL == arg)
 		return NULL;
@@ -23,11 +24,16 @@ LPSTR tr_esc_str(LPCSTR arg, bool format)
 
 	/* Prepare a initial buffer with the size of the result string. */
 	ds = s + 1;
 
 	if (s)
-		tmp = (LPSTR)realloc(tmp, ds * sizeof(CHAR));
+	{
+		tmp2 = (LPSTR)realloc(tmp, ds * sizeof(CHAR));
+		if (!tmp2)
+			free(tmp);
+		tmp = tmp2;
+	}
 
 	if (NULL == tmp)
 	{
 		fprintf(stderr,  "Could not allocate string buffer.\n");
 		exit(-2);
@@ -41,11 +47,14 @@ LPSTR tr_esc_str(LPCSTR arg, bool format)
 		switch (arg[x])
 		{
 			case '<':
 				len = format ? 13 : 4;
 				ds += len - 1;
-				tmp = (LPSTR)realloc(tmp, ds * sizeof(CHAR));
+				tmp2 = (LPSTR)realloc(tmp, ds * sizeof(CHAR));
+				if (!tmp2)
+					free(tmp);
+				tmp = tmp2;
 
 				if (NULL == tmp)
 				{
 					fprintf(stderr,  "Could not reallocate string buffer.\n");
 					exit(-3);
@@ -62,11 +71,14 @@ LPSTR tr_esc_str(LPCSTR arg, bool format)
 				break;
 
 			case '>':
 				len = format ? 14 : 4;
 				ds += len - 1;
-				tmp = (LPSTR)realloc(tmp, ds * sizeof(CHAR));
+				tmp2 = (LPSTR)realloc(tmp, ds * sizeof(CHAR));
+				if (!tmp2)
+					free(tmp);
+				tmp = tmp2;
 
 				if (NULL == tmp)
 				{
 					fprintf(stderr,  "Could not reallocate string buffer.\n");
 					exit(-4);
@@ -82,11 +94,14 @@ LPSTR tr_esc_str(LPCSTR arg, bool format)
 				cs += len;
 				break;
 
 			case '\'':
 				ds += 5;
-				tmp = (LPSTR)realloc(tmp, ds * sizeof(CHAR));
+				tmp2 = (LPSTR)realloc(tmp, ds * sizeof(CHAR));
+				if (!tmp2)
+					free(tmp);
+				tmp = tmp2;
 
 				if (NULL == tmp)
 				{
 					fprintf(stderr,  "Could not reallocate string buffer.\n");
 					exit(-5);
@@ -100,11 +115,14 @@ LPSTR tr_esc_str(LPCSTR arg, bool format)
 				tmp[cs++] = ';';
 				break;
 
 			case '"':
 				ds += 5;
-				tmp = (LPSTR)realloc(tmp, ds * sizeof(CHAR));
+				tmp2 = (LPSTR)realloc(tmp, ds * sizeof(CHAR));
+				if (!tmp2)
+					free(tmp);
+				tmp = tmp2;
 
 				if (NULL == tmp)
 				{
 					fprintf(stderr,  "Could not reallocate string buffer.\n");
 					exit(-6);
@@ -118,11 +136,14 @@ LPSTR tr_esc_str(LPCSTR arg, bool format)
 				tmp[cs++] = ';';
 				break;
 
 			case '&':
 				ds += 4;
-				tmp = (LPSTR)realloc(tmp, ds * sizeof(CHAR));
+				tmp2 = (LPSTR)realloc(tmp, ds * sizeof(CHAR));
+				if (!tmp2)
+					free(tmp);
+				tmp = tmp2;
 
 				if (NULL == tmp)
 				{
 					fprintf(stderr,  "Could not reallocate string buffer.\n");
 					exit(-7);
diff --git a/libfreerdp/codec/region.c b/libfreerdp/codec/region.c
index 2bc866538c..c5d19c8061 100644
--- a/libfreerdp/codec/region.c
+++ b/libfreerdp/codec/region.c
@@ -465,12 +465,16 @@ static BOOL region16_simplify_bands(REGION16* region)
 	}
 	while (TRUE);
 
 	if (finalNbRects != nbRects)
 	{
-		int allocSize = sizeof(REGION16_DATA) + (finalNbRects * sizeof(RECTANGLE_16));
-		region->data = realloc(region->data, allocSize);
+		REGION16_DATA* data;
+		size_t allocSize = sizeof(REGION16_DATA) + (finalNbRects * sizeof(RECTANGLE_16));
+		data = realloc(region->data, allocSize);
+		if (!data)
+			free(region->data);
+		region->data = data;
 
 		if (!region->data)
 		{
 			region->data = &empty_region;
 			return FALSE;
@@ -483,14 +487,16 @@ static BOOL region16_simplify_bands(REGION16* region)
 	return TRUE;
 }
 
 BOOL region16_union_rect(REGION16* dst, const REGION16* src, const RECTANGLE_16* rect)
 {
+	REGION16_DATA* data;
 	const RECTANGLE_16* srcExtents;
 	RECTANGLE_16* dstExtents;
 	const RECTANGLE_16* currentBand, *endSrcRect, *nextBand;
 	REGION16_DATA* newItems = NULL;
+	REGION16_DATA* tmpItems = NULL;
 	RECTANGLE_16* dstRect = NULL;
 	UINT32 usedRects, srcNbRects;
 	UINT16 topInterBand;
 	assert(src);
 	assert(src->data);
@@ -671,11 +677,15 @@ BOOL region16_union_rect(REGION16* dst, const REGION16* src, const RECTANGLE_16*
 	dstExtents->top = MIN(rect->top, srcExtents->top);
 	dstExtents->left = MIN(rect->left, srcExtents->left);
 	dstExtents->bottom = MAX(rect->bottom, srcExtents->bottom);
 	dstExtents->right = MAX(rect->right, srcExtents->right);
 	newItems->size = sizeof(REGION16_DATA) + (usedRects * sizeof(RECTANGLE_16));
-	dst->data = realloc(newItems, newItems->size);
+	tmpItems = realloc(newItems, newItems->size);
+	if (!tmpItems)
+		free(newItems);
+	newItems = tmpItems;
+	dst->data = newItems;
 
 	if (!dst->data)
 	{
 		free(newItems);
 		return FALSE;
@@ -715,10 +725,11 @@ BOOL region16_intersects_rect(const REGION16* src, const RECTANGLE_16* arg2)
 	return FALSE;
 }
 
 BOOL region16_intersect_rect(REGION16* dst, const REGION16* src, const RECTANGLE_16* rect)
 {
+	REGION16_DATA* data;
 	REGION16_DATA* newItems;
 	const RECTANGLE_16* srcPtr, *endPtr, *srcExtents;
 	RECTANGLE_16* dstPtr;
 	UINT32 nbRects, usedRects;
 	RECTANGLE_16 common, newExtents;
diff --git a/winpr/libwinpr/utils/lodepng/lodepng.c b/winpr/libwinpr/utils/lodepng/lodepng.c
index 741a953b84..b48c881a2d 100644
--- a/winpr/libwinpr/utils/lodepng/lodepng.c
+++ b/winpr/libwinpr/utils/lodepng/lodepng.c
@@ -839,15 +839,19 @@ unsigned lodepng_huffman_code_lengths(unsigned* lengths, const unsigned* frequen
 
 /*Create the Huffman tree given the symbol frequencies*/
 static unsigned HuffmanTree_makeFromFrequencies(HuffmanTree* tree, const unsigned* frequencies,
                                                 size_t mincodes, size_t numcodes, unsigned maxbitlen)
 {
+	unsigned* lengths;
   unsigned error = 0;
   while(!frequencies[numcodes - 1] && numcodes > mincodes) numcodes--; /*trim zeroes*/
   tree->maxbitlen = maxbitlen;
   tree->numcodes = (unsigned)numcodes; /*number of symbols*/
-  tree->lengths = (unsigned*)realloc(tree->lengths, numcodes * sizeof(unsigned));
+  lengths = (unsigned*)realloc(tree->lengths, numcodes * sizeof(unsigned));
+	if (!lengths)
+		free(tree->lengths);
+	tree->lengths = lengths;
   if(!tree->lengths) return 83; /*alloc fail*/
   /*initialize all lengths to 0*/
   memset(tree->lengths, 0, numcodes * sizeof(unsigned));
 
   error = lodepng_huffman_code_lengths(tree->lengths, frequencies, numcodes, maxbitlen);

Vulnerable code: 		tmp = (LPSTR)realloc(tmp, ds * sizeof(CHAR));
				tmp = (LPSTR)realloc(tmp, ds * sizeof(CHAR));
				tmp = (LPSTR)realloc(tmp, ds * sizeof(CHAR));
				tmp = (LPSTR)realloc(tmp, ds * sizeof(CHAR));
				tmp = (LPSTR)realloc(tmp, ds * sizeof(CHAR));
				tmp = (LPSTR)realloc(tmp, ds * sizeof(CHAR));
		int allocSize = sizeof(REGION16_DATA) + (finalNbRects * sizeof(RECTANGLE_16));
		region->data = realloc(region->data, allocSize);
	dst->data = realloc(newItems, newItems->size);
  tree->lengths = (unsigned*)realloc(tree->lengths, numcodes * sizeof(unsigned));
Fixed code: 	LPSTR tmp2 = NULL;
	{
		tmp2 = (LPSTR)realloc(tmp, ds * sizeof(CHAR));
		if (!tmp2)
			free(tmp);
		tmp = tmp2;
	}
				tmp2 = (LPSTR)realloc(tmp, ds * sizeof(CHAR));
				if (!tmp2)
					free(tmp);
				tmp = tmp2;
				tmp2 = (LPSTR)realloc(tmp, ds * sizeof(CHAR));
				if (!tmp2)
					free(tmp);
				tmp = tmp2;
				tmp2 = (LPSTR)realloc(tmp, ds * sizeof(CHAR));
				if (!tmp2)
					free(tmp);
				tmp = tmp2;
				tmp2 = (LPSTR)realloc(tmp, ds * sizeof(CHAR));
				if (!tmp2)
					free(tmp);
				tmp = tmp2;
				tmp2 = (LPSTR)realloc(tmp, ds * sizeof(CHAR));
				if (!tmp2)
					free(tmp);
				tmp = tmp2;
		REGION16_DATA* data;
		size_t allocSize = sizeof(REGION16_DATA) + (finalNbRects * sizeof(RECTANGLE_16));
		data = realloc(region->data, allocSize);
		if (!data)
			free(region->data);
		region->data = data;
	REGION16_DATA* data;
	REGION16_DATA* tmpItems = NULL;
	tmpItems = realloc(newItems, newItems->size);
	if (!tmpItems)
		free(newItems);
	newItems = tmpItems;
	dst->data = newItems;
	REGION16_DATA* data;
	unsigned* lengths;
  lengths = (unsigned*)realloc(tree->lengths, numcodes * sizeof(unsigned));
	if (!lengths)
		free(tree->lengths);
	tree->lengths = lengths;