git name-rev writes beyond the end of malloc() with large generations

When using git name-rev on my kernel tree I triggered a malloc()
corruption warning from glibc.

apw@pinky$ git log --pretty=one $N/base.. | git name-rev --stdin
*** glibc detected *** malloc(): memory corruption: 0x0bff8950 ***
Aborted

This comes from name_rev() which is building the name of the revision
in a malloc'd string, which it sprintf's into:

	char *new_name = xmalloc(len + 8);
	[...]
		sprintf(new_name, "%.*s~%d^%d", len, tip_name,
				generation, parent_number);

This allocation is only sufficient if the generation number is
less than 5 digits, in my case generation was 13432.  In reality
parent_number can be up to 16 so that also can require two digits,
reducing us to 3 digits before we are at risk of blowing this
allocation.

This patch introduces a decimal_length() which approximates the
number of digits a type may hold, it produces the following:

Type                 Longest Value          Len  Est
----                 -------------          ---  ---
unsigned char        256                      3    4
unsigned short       65536                    5    6
unsigned long        4294967296              10   11
unsigned long long   18446744073709551616    20   21
char                 -128                     4    4
short                -32768                   6    6
long                 -2147483648             11   11
long long            -9223372036854775808    20   21

This is then used to size the new_name.

Signed-off-by: Andy Whitcroft <apw@shadowen.org>
Signed-off-by: Junio C Hamano <junkio@cox.net>

diff --git a/builtin-name-rev.c b/builtin-name-rev.c
index c022224361b62b527aca7822920ac8ea896ae585..ef1638590722017ad2cacca7ce30098fd0392bd5 100644 (file)
--- a/builtin-name-rev.c
+++ b/builtin-name-rev.c
@@ -58,7 +58,10 @@ copy_data:
                        parents = parents->next, parent_number++) {
                if (parent_number > 1) {
                        int len = strlen(tip_name);
-                       char *new_name = xmalloc(len + 8);
+                       char *new_name = xmalloc(len +
+                               1 + decimal_length(generation) +  /* ~<n> */
+                               1 + 2 +                           /* ^NN */
+                               1);
 
                        if (len > 2 && !strcmp(tip_name + len - 2, "^0"))
                                len -= 2;

diff --git a/git-compat-util.h b/git-compat-util.h
index bd93b6257885d56fda7268ae88ea82f721a6707f..7ed8b88b1f3cef2562693bbf6d68e2e4cef9465b 100644 (file)
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -13,6 +13,9 @@
 
 #define ARRAY_SIZE(x) (sizeof(x)/sizeof(x[0]))
 
+/* Approximation of the length of the decimal representation of this type. */
+#define decimal_length(x)      ((int)(sizeof(x) * 2.56 + 0.5) + 1)
+
 #if !defined(__APPLE__) && !defined(__FreeBSD__)
 #define _XOPEN_SOURCE 600 /* glibc2 and AIX 5.3L need 500, OpenBSD needs 600 for S_ISLNK() */
 #define _XOPEN_SOURCE_EXTENDED 1 /* AIX 5.3L needs this */