Commit c8e94f84 by Patrick Steinhardt

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.

(cherry picked from commit ea19efc1)
parent 0f7663a1
...@@ -162,20 +162,24 @@ Return: ...@@ -162,20 +162,24 @@ Return:
int git__strntol32(int32_t *result, const char *nptr, size_t nptr_len, const char **endptr, int base) 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; int32_t tmp_int;
int64_t tmp_long; 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; return error;
tmp_int = tmp_long & 0xFFFFFFFF; tmp_int = tmp_long & 0xFFFFFFFF;
if (tmp_int != tmp_long) { 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; return -1;
} }
*result = tmp_int; *result = tmp_int;
if (endptr)
*endptr = tmp_endptr;
return error; return error;
} }
......
...@@ -75,3 +75,10 @@ void test_core_strtol__buffer_length_truncates(void) ...@@ -75,3 +75,10 @@ void test_core_strtol__buffer_length_truncates(void)
cl_git_pass(git__strntol64(&i64, "11", 1, NULL, 10)); cl_git_pass(git__strntol64(&i64, "11", 1, NULL, 10));
cl_assert_equal_i(i64, 1); 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);
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment