diff options
author | Patrick Steinhardt <ps@pks.im> | 2018-10-18 15:08:56 +0200 |
---|---|---|
committer | Patrick Steinhardt <ps@pks.im> | 2018-10-19 12:18:30 +0200 |
commit | ea19efc19fa683d632af3e172868bc4350724813 (patch) | |
tree | af92375e31236d217126aaeaf8748fd832ad507d | |
parent | b09c1c7b636c4112e247adc24245c65f3f9478d0 (diff) | |
download | libgit2-ea19efc19fa683d632af3e172868bc4350724813.tar.gz |
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.
-rw-r--r-- | src/util.c | 10 | ||||
-rw-r--r-- | tests/core/strtol.c | 7 |
2 files changed, 14 insertions, 3 deletions
diff --git a/src/util.c b/src/util.c index a1336f3a5..20c88a1d3 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 3d284b350..ba79fba51 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); +} |