Properly check `strtol` for errors

We are now using a custom `strtol` implementation to make sure we're not
missing any overflow errors.

8 files changed, 118 insertions, 15 deletions

diff --git a/include/git2/common.h b/include/git2/common.h
index e0c6bc542..22c7cc466 100644
--- a/include/git2/common.h
+++ b/include/git2/common.h
@@ -164,6 +164,12 @@
 /** A reference with this name already exists */
 #define GIT_EEXISTS (GIT_ERROR - 23)
 
+/** The given integer literal is too large to be parsed */
+#define GIT_EOVERFLOW (GIT_ERROR - 24)
+
+/** The given literal is not a valid number */
+#define GIT_ENOTNUM (GIT_ERROR - 25)
+
 GIT_BEGIN_DECL
 
 typedef struct {
diff --git a/src/errors.c b/src/errors.c
index 3c0e8eb1f..c3a495cc9 100644
--- a/src/errors.c
+++ b/src/errors.c
@@ -29,7 +29,9 @@ static struct {
 	{GIT_EREVWALKOVER, "The revision walker is empty; there are no more commits left to iterate"},
 	{GIT_EINVALIDREFSTATE, "The state of the reference is not valid"},
 	{GIT_ENOTIMPLEMENTED, "This feature has not been implemented yet"},
-	{GIT_EEXISTS, "A reference with this name already exists"}
+	{GIT_EEXISTS, "A reference with this name already exists"},
+	{GIT_EOVERFLOW, "The given integer literal is too large to be parsed"},
+	{GIT_ENOTNUM, "The given literal is not a valid number"},
 };
 
 const char *git_strerror(int num)
diff --git a/src/index.c b/src/index.c
index 6a31dd5cb..68bb9e2b9 100644
--- a/src/index.c
+++ b/src/index.c
@@ -411,6 +411,7 @@ static git_index_tree *read_tree_internal(
 {
 	git_index_tree *tree;
 	const char *name_start, *buffer;
+	long count;
 
 	if ((tree = git__malloc(sizeof(git_index_tree))) == NULL)
 		return NULL;
@@ -429,12 +430,22 @@ static git_index_tree *read_tree_internal(
 		goto error_cleanup;
 
 	/* Blank-terminated ASCII decimal number of entries in this tree */
-	tree->entries = strtol(buffer, (char **)&buffer, 10);
+	if (git__strtol32(&count, buffer, &buffer, 10) < GIT_SUCCESS ||
+		count < 0)
+		goto error_cleanup;
+
+	tree->entries = (size_t)count;
+
 	if (*buffer != ' ' || ++buffer >= buffer_end)
 		goto error_cleanup;
 
 	 /* Number of children of the tree, newline-terminated */
-	tree->children_count = strtol(buffer, (char **)&buffer, 10);
+	if (git__strtol32(&count, buffer, &buffer, 10) < GIT_SUCCESS ||
+		count < 0)
+		goto error_cleanup;
+
+	tree->children_count = (size_t)count;
+
 	if (*buffer != '\n' || ++buffer >= buffer_end)
 		goto error_cleanup;
 
diff --git a/src/revwalk.c b/src/revwalk.c
index 73bb060f5..b62b09961 100644
--- a/src/revwalk.c
+++ b/src/revwalk.c
@@ -191,6 +191,7 @@ static int commit_quick_parse(git_revwalk *walk, commit_object *commit, git_rawo
 	unsigned char *parents_start;
 
 	int i, parents = 0;
+	long commit_time;
 
 	buffer += STRLEN("tree ") + GIT_OID_HEXSZ + 1;
 
@@ -227,10 +228,10 @@ static int commit_quick_parse(git_revwalk *walk, commit_object *commit, git_rawo
 	if (buffer == NULL)
 		return GIT_EOBJCORRUPTED;
 
-	commit->time = strtol((char *)buffer + 2, NULL, 10);
-	if (commit->time == 0)
+	if (git__strtol32(&commit_time, (char *)buffer + 2, NULL, 10) < GIT_SUCCESS)
 		return GIT_EOBJCORRUPTED;
 
+	commit->time = (time_t)commit_time;
 	commit->parsed = 1;
 	return GIT_SUCCESS;
 }
diff --git a/src/signature.c b/src/signature.c
index 0c99755d4..7c4397922 100644
--- a/src/signature.c
+++ b/src/signature.c
@@ -66,13 +66,13 @@ git_signature *git_signature_dup(const git_signature *sig)
 }
 
 
-static int parse_timezone_offset(const char *buffer, int *offset_out)
+static int parse_timezone_offset(const char *buffer, long *offset_out)
 {
-	int offset, dec_offset;
+	long offset, dec_offset;
 	int mins, hours;
 
-	const char* offset_start;
-	char* offset_end;
+	const char *offset_start;
+	const char *offset_end;
 
 	offset_start = buffer + 1;
 
@@ -84,7 +84,8 @@ static int parse_timezone_offset(const char *buffer, int *offset_out)
 	if (offset_start[0] != '-' && offset_start[0] != '+')
 		return GIT_EOBJCORRUPTED;
 
-	dec_offset = strtol(offset_start + 1, &offset_end, 10);
+	if (git__strtol32(&dec_offset, offset_start + 1, &offset_end, 10) < GIT_SUCCESS)
+		return GIT_EOBJCORRUPTED;
 
 	if (offset_end - offset_start != 5)
 		return GIT_EOBJCORRUPTED;
@@ -117,7 +118,7 @@ int git_signature__parse(git_signature *sig, const char **buffer_out,
 	int name_length, email_length;
 	const char *buffer = *buffer_out;
 	const char *line_end, *name_end, *email_end;
-	int offset = 0;
+	long offset = 0, time;
 
 	memset(sig, 0x0, sizeof(git_signature));
 
@@ -159,11 +160,11 @@ int git_signature__parse(git_signature *sig, const char **buffer_out,
 	if (buffer >= line_end)
 		return GIT_EOBJCORRUPTED;
 
-	sig->when.time = strtol(buffer, (char **)&buffer, 10);
-
-	if (sig->when.time == 0)
+	if (git__strtol32(&time, buffer, &buffer, 10) < GIT_SUCCESS)
 		return GIT_EOBJCORRUPTED;
 
+	sig->when.time = (time_t)time;
+
 	if (parse_timezone_offset(buffer, &offset) < GIT_SUCCESS)
 		return GIT_EOBJCORRUPTED;
 	
diff --git a/src/tree.c b/src/tree.c
index b474d394b..64f81d780 100644
--- a/src/tree.c
+++ b/src/tree.c
@@ -150,7 +150,8 @@ static int tree_parse_buffer(git_tree *tree, const char *buffer, const char *buf
 		if (git_vector_insert(&tree->entries, entry) < GIT_SUCCESS)
 			return GIT_ENOMEM;
 
-		entry->attr = strtol(buffer, (char **)&buffer, 8);
+		if (git__strtol32((long *)&entry->attr, buffer, &buffer, 8) < GIT_SUCCESS)
+			return GIT_EOBJCORRUPTED;
 
 		if (*buffer++ != ' ') {
 			error = GIT_EOBJCORRUPTED;
diff --git a/src/util.c b/src/util.c
index 995daf314..55a7ab2a9 100644
--- a/src/util.c
+++ b/src/util.c
@@ -2,6 +2,7 @@
 #include "common.h"
 #include <stdarg.h>
 #include <stdio.h>
+#include <ctype.h>
 
 void git_strarray_free(git_strarray *array)
 {
@@ -12,6 +13,84 @@ void git_strarray_free(git_strarray *array)
 	free(array->strings);
 }
 
+int git__strtol32(long *result, const char *nptr, const char **endptr, int base)
+{
+	const char *p;
+	long n, nn;
+	int c, ovfl, v, neg, ndig;
+
+	p = nptr;
+	neg = 0;
+	n = 0;
+	ndig = 0;
+	ovfl = 0;
+
+	/*
+	 * White space
+	 */
+	while (isspace(*p))
+		p++;
+
+	/*
+	 * Sign
+	 */
+	if (*p == '-' || *p == '+')
+		if (*p++ == '-')
+			neg = 1;
+
+	/*
+	 * Base
+	 */
+	if (base == 0) {
+		if (*p != '0')
+			base = 10;
+		else {
+			base = 8;
+			if (p[1] == 'x' || p[1] == 'X') {
+				p += 2;
+				base = 16;
+			}
+		}
+	} else if (base == 16 && *p == '0') {
+		if (p[1] == 'x' || p[1] == 'X')
+			p += 2;
+	} else if (base < 0 || 36 < base)
+		goto Return;
+
+	/*
+	 * Non-empty sequence of digits
+	 */
+	for (;; p++,ndig++) {
+		c = *p;
+		v = base;
+		if ('0'<=c && c<='9')
+			v = c - '0';
+		else if ('a'<=c && c<='z')
+			v = c - 'a' + 10;
+		else if ('A'<=c && c<='Z')
+			v = c - 'A' + 10;
+		if (v >= base)
+			break;
+		nn = n*base + v;
+		if (nn < n)
+			ovfl = 1;
+		n = nn;
+	}
+
+Return:
+	if (ndig == 0)
+		return GIT_ENOTNUM;
+
+	if (endptr)
+		*endptr = p;
+
+	if (ovfl)
+		return GIT_EOVERFLOW;
+
+	*result = neg ? -n : n;
+	return GIT_SUCCESS;
+}
+
 int git__fmt(char *buf, size_t buf_sz, const char *fmt, ...)
 {
 	va_list va;
diff --git a/src/util.h b/src/util.h
index f477b64fd..3c606493f 100644
--- a/src/util.h
+++ b/src/util.h
@@ -22,6 +22,8 @@ extern int git__fmt(char *, size_t, const char *, ...)
 extern int git__prefixcmp(const char *str, const char *prefix);
 extern int git__suffixcmp(const char *str, const char *suffix);
 
+extern int git__strtol32(long *n, const char *buff, const char **end_buf, int base);
+
 /*
  * The dirname() function shall take a pointer to a character string
  * that contains a pathname, and return a pointer to a string that is a