harden REALLOC_ARRAY and xcalloc against size_t overflow

peff · gitster · commit e7792a74bcf7 · 2016-02-22T14:50:32.000-08:00
REALLOC_ARRAY inherently involves a multiplication which can
overflow size_t, resulting in a much smaller buffer than we
think we've allocated. We can easily harden it by using
st_mult() to check for overflow.  Likewise, we can add
ALLOC_ARRAY to do the same thing for xmalloc calls.

xcalloc() should already be fine, because it takes the two
factors separately, assuming the system calloc actually
checks for overflow. However, before we even hit the system
calloc(), we do our memory_limit_check, which involves a
multiplication. Let's check for overflow ourselves so that
this limit cannot be bypassed.

Signed-off-by: Jeff King &lt;peff@peff.net&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
diff --git a/git-compat-util.h b/git-compat-util.h
@@ -779,7 +779,8 @@ extern int odb_pack_keep(char *name, size_t namesz, const unsigned char *sha1);
 extern char *xgetcwd(void);
 extern FILE *fopen_for_writing(const char *path);
 
-#define REALLOC_ARRAY(x, alloc) (x) = xrealloc((x), (alloc) * sizeof(*(x)))
+#define ALLOC_ARRAY(x, alloc) (x) = xmalloc(st_mult(sizeof(*(x)), (alloc)))
+#define REALLOC_ARRAY(x, alloc) (x) = xrealloc((x), st_mult(sizeof(*(x)), (alloc)))
 
 static inline char *xstrdup_or_null(const char *str)
 {
diff --git a/wrapper.c b/wrapper.c
@@ -152,6 +152,9 @@ void *xcalloc(size_t nmemb, size_t size)
 {
 	void *ret;
 
+	if (unsigned_mult_overflows(nmemb, size))
+		die("data too large to fit into virtual memory space");
+
 	memory_limit_check(size * nmemb, 0);
 	ret = calloc(nmemb, size);
 	if (!ret && (!nmemb || !size))

Original file line number	Diff line number	Diff line change
`@@ -779,7 +779,8 @@ extern int odb_pack_keep(char name, size_t namesz, const unsigned char sha1);`
`779`	`779`	`extern char *xgetcwd(void);`
`780`	`780`	`extern FILE fopen_for_writing(const char path);`
`781`	`781`
`782`		`-#define REALLOC_ARRAY(x, alloc) (x) = xrealloc((x), (alloc) * sizeof(*(x)))`
	`782`	`+#define ALLOC_ARRAY(x, alloc) (x) = xmalloc(st_mult(sizeof(*(x)), (alloc)))`
	`783`	`+#define REALLOC_ARRAY(x, alloc) (x) = xrealloc((x), st_mult(sizeof(*(x)), (alloc)))`
`783`	`784`
`784`	`785`	`static inline char xstrdup_or_null(const char str)`
`785`	`786`	`{`
Original file line number	Diff line number	Diff line change
`@@ -152,6 +152,9 @@ void *xcalloc(size_t nmemb, size_t size)`
`152`	`152`	`{`
`153`	`153`	`void *ret;`
`154`	`154`
	`155`	`+ if (unsigned_mult_overflows(nmemb, size))`
	`156`	`+ die("data too large to fit into virtual memory space");`
	`157`	`+`
`155`	`158`	`memory_limit_check(size * nmemb, 0);`
`156`	`159`	`ret = calloc(nmemb, size);`
`157`	`160`	`if (!ret && (!nmemb \|\| !size))`