From: Tobias Stoeckmann <tobias@stoeckmann.org>
Date: Thu, 24 Feb 2022 20:35:44 +0000 (+0100)
Subject: vasprintf(): avoid out of memory accesses
X-Git-Tag: json-c-0.16-20220414~17^2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=refs%2Fpull%2F745%2Fhead;p=thirdparty%2Fjson-c.git

vasprintf(): avoid out of memory accesses

Systems without vasprintf fall back to implementation in header file
vasprintf_compat.h. This version could run into heap overflow issues
with very long arguments or formats provoking a lot of output.

The vsnprintf function returns a negative value if more than INT_MAX
characters would be written since its int return type could not
handle this (and %n couldn't handle it either).

Before testing for a possible error value the additional char for
\0 is already added. A -1 error code would not be detected.

Increment only after implicitly casting to an unsigned value to avoid
signed integer overflow if INT_MAX has been returned.

Use va_copy to duplicate the original ap argument for multiple uses
on non-WIN32 systems. At least with glibc the test suite would fail
because the arguments are not reset after leaving the vsnprintf call.

Removed support for apparently very old glibc versions which do not
comply with vsnprintf standard descriptions. It breaks support for
modern ones which are not forced to return -1 in case of error. The
standard specifies merely "a negative value".

How to reproduce:

- Use a system without vasprintf
- Alternatively remove -D_GNU_SOURCE from CMakeLists.txt
- Compile and run:

  #include "json.h"
  int main(void) {
    struct printbuf *pb = printbuf_new();
    sprintbuf(pb, "prefix %2147483647s", "*");
    printbuf_free(pb);
    return 0;
  }
---

diff --git a/vasprintf_compat.h b/vasprintf_compat.h
index 52642723..59b2e960 100644
--- a/vasprintf_compat.h
+++ b/vasprintf_compat.h
@@ -8,6 +8,10 @@
 
 #include "snprintf_compat.h"
 
+#ifndef WIN32
+#include <stdarg.h>
+#endif /* !defined(WIN32) */
+#include <stdint.h>
 #include <stdlib.h>
 
 #if !defined(HAVE_VASPRINTF)
@@ -16,6 +20,7 @@ static int vasprintf(char **buf, const char *fmt, va_list ap)
 {
 #ifndef WIN32
 	static char _T_emptybuffer = '\0';
+	va_list ap2;
 #endif /* !defined(WIN32) */
 	int chars;
 	char *b;
@@ -26,19 +31,21 @@ static int vasprintf(char **buf, const char *fmt, va_list ap)
 	}
 
 #ifdef WIN32
-	chars = _vscprintf(fmt, ap) + 1;
+	chars = _vscprintf(fmt, ap);
 #else  /* !defined(WIN32) */
 	/* CAW: RAWR! We have to hope to god here that vsnprintf doesn't overwrite
-	 * our buffer like on some 64bit sun systems.... but hey, its time to move on
+	 * our buffer like on some 64bit sun systems... but hey, it's time to move on
 	 */
-	chars = vsnprintf(&_T_emptybuffer, 0, fmt, ap) + 1;
-	if (chars < 0)
-	{
-		chars *= -1;
-	} /* CAW: old glibc versions have this problem */
+	va_copy(ap2, ap);
+	chars = vsnprintf(&_T_emptybuffer, 0, fmt, ap2);
+	va_end(ap2);
 #endif /* defined(WIN32) */
+	if (chars < 0 || (size_t)chars + 1 > SIZE_MAX / sizeof(char))
+	{
+		return -1;
+	}
 
-	b = (char *)malloc(sizeof(char) * chars);
+	b = (char *)malloc(sizeof(char) * ((size_t)chars + 1));
 	if (!b)
 	{
 		return -1;