Commit ea19efc19fa683d632af3e172868bc4350724813
2018-10-18T15:08:56
util: fix out of bounds read in error message When an integer that is parsed with `git__strntol32` is too big to fit into an int32, we will generate an error message that includes the actual string that failed to parse. This does not acknowledge the fact that the string may either not be NUL terminated or alternative include additional characters after the number that is to be parsed. We may thus end up printing characters into the buffer that aren't the number or, worse, read out of bounds. Fix the issue by utilizing the `endptr` that was set by `git__strntol64`. This pointer is guaranteed to be set to the first character following the number, and we can thus use it to compute the width of the number that shall be printed. Create a test to verify that we correctly truncate the number.
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47
diff --git a/src/util.c b/src/util.c
index a1336f3..20c88a1 100644
--- a/src/util.c
+++ b/src/util.c
@@ -162,20 +162,24 @@ Return:
int git__strntol32(int32_t *result, const char *nptr, size_t nptr_len, const char **endptr, int base)
{
- int error;
+ const char *tmp_endptr;
int32_t tmp_int;
int64_t tmp_long;
+ int error;
- if ((error = git__strntol64(&tmp_long, nptr, nptr_len, endptr, base)) < 0)
+ if ((error = git__strntol64(&tmp_long, nptr, nptr_len, &tmp_endptr, base)) < 0)
return error;
tmp_int = tmp_long & 0xFFFFFFFF;
if (tmp_int != tmp_long) {
- giterr_set(GITERR_INVALID, "failed to convert: '%s' is too large", nptr);
+ int len = tmp_endptr - nptr;
+ giterr_set(GITERR_INVALID, "failed to convert: '%.*s' is too large", len, nptr);
return -1;
}
*result = tmp_int;
+ if (endptr)
+ *endptr = tmp_endptr;
return error;
}
diff --git a/tests/core/strtol.c b/tests/core/strtol.c
index 3d284b3..ba79fba 100644
--- a/tests/core/strtol.c
+++ b/tests/core/strtol.c
@@ -75,3 +75,10 @@ void test_core_strtol__buffer_length_truncates(void)
cl_git_pass(git__strntol64(&i64, "11", 1, NULL, 10));
cl_assert_equal_i(i64, 1);
}
+
+void test_core_strtol__error_message_cuts_off(void)
+{
+ assert_l32_fails("2147483657foobar", 10);
+ cl_assert(strstr(giterr_last()->message, "2147483657") != NULL);
+ cl_assert(strstr(giterr_last()->message, "foobar") == NULL);
+}