Merge pull request #4886 from pks-t/pks/strntol-truncate-leading-sign

strntol: fix out-of-bounds reads when parsing numbers with leading sign

3 files changed, 18 insertions, 22 deletions

diff --git a/src/patch_parse.c b/src/patch_parse.c
index f9dcecd1c..0244dbb52 100644
--- a/src/patch_parse.c
+++ b/src/patch_parse.c
@@ -458,26 +458,6 @@ done:
 	return error;
 }
 
-static int parse_number(git_off_t *out, git_patch_parse_ctx *ctx)
-{
-	const char *end;
-	int64_t num;
-
-	if (!git__isdigit(ctx->parse_ctx.line[0]))
-		return -1;
-
-	if (git__strntol64(&num, ctx->parse_ctx.line, ctx->parse_ctx.line_len, &end, 10) < 0)
-		return -1;
-
-	if (num < 0)
-		return -1;
-
-	*out = num;
-	git_parse_advance_chars(&ctx->parse_ctx, (end - ctx->parse_ctx.line));
-
-	return 0;
-}
-
 static int parse_int(int *out, git_patch_parse_ctx *ctx)
 {
 	git_off_t num;
diff --git a/src/util.c b/src/util.c
index 735f0b547..9952a60fe 100644
--- a/src/util.c
+++ b/src/util.c
@@ -92,9 +92,15 @@ int git__strntol64(int64_t *result, const char *nptr, size_t nptr_len, const cha
 	/*
 	 * Sign
 	 */
-	if (*p == '-' || *p == '+')
-		if (*p++ == '-')
+	if (*p == '-' || *p == '+') {
+		if (*p == '-')
 			neg = 1;
+		p++;
+		nptr_len--;
+	}
+
+	if (!nptr_len)
+		goto Return;
 
 	/*
 	 * Automatically detect the base if none was given to us.
diff --git a/tests/core/strtol.c b/tests/core/strtol.c
index 71af3325e..6f4e63af3 100644
--- a/tests/core/strtol.c
+++ b/tests/core/strtol.c
@@ -108,6 +108,16 @@ void test_core_strtol__buffer_length_with_leading_ws_truncates(void)
 	cl_assert_equal_i(i64, 1);
 }
 
+void test_core_strtol__buffer_length_with_leading_sign_truncates(void)
+{
+	int64_t i64;
+
+	cl_git_fail(git__strntol64(&i64, "-1", 1, NULL, 10));
+
+	cl_git_pass(git__strntol64(&i64, "-11", 2, NULL, 10));
+	cl_assert_equal_i(i64, -1);
+}
+
 void test_core_strtol__error_message_cuts_off(void)
 {
 	assert_l32_fails("2147483657foobar", 10);