Commit 63e9c7fe 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 737df8b5
......@@ -158,20 +158,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;
}
......
......@@ -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);
}
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