summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPatrick Steinhardt <ps@pks.im>2018-10-18 15:08:56 +0200
committerPatrick Steinhardt <ps@pks.im>2018-10-19 12:18:30 +0200
commitea19efc19fa683d632af3e172868bc4350724813 (patch)
treeaf92375e31236d217126aaeaf8748fd832ad507d
parentb09c1c7b636c4112e247adc24245c65f3f9478d0 (diff)
downloadlibgit2-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.c10
-rw-r--r--tests/core/strtol.c7
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);
+}