From: Andy Whitcroft <apw@shadowen.org>
Date: Tue, 15 May 2007 16:33:25 +0000 (+0100)
Subject: git name-rev writes beyond the end of malloc() with large generations
X-Git-Tag: v1.5.1.5~8
X-Git-Url: https://git.tokkee.org/?a=commitdiff_plain;h=cf606e3ddd8666b990a6560be77eb9f28af0e47d;p=git.git

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 c02222436..ef1638590 100644
--- 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 bd93b6257..7ed8b88b1 100644
--- 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 */