Skip to content

Commit ea19efc

Browse files
committed
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 parent b09c1c7 commit ea19efc

File tree

2 files changed

+14
-3
lines changed

2 files changed

+14
-3
lines changed

src/util.c

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -162,20 +162,24 @@ int git__strntol64(int64_t *result, const char *nptr, size_t nptr_len, const cha
162162

163163
int git__strntol32(int32_t *result, const char *nptr, size_t nptr_len, const char **endptr, int base)
164164
{
165-
int error;
165+
const char *tmp_endptr;
166166
int32_t tmp_int;
167167
int64_t tmp_long;
168+
int error;
168169

169-
if ((error = git__strntol64(&tmp_long, nptr, nptr_len, endptr, base)) < 0)
170+
if ((error = git__strntol64(&tmp_long, nptr, nptr_len, &tmp_endptr, base)) < 0)
170171
return error;
171172

172173
tmp_int = tmp_long & 0xFFFFFFFF;
173174
if (tmp_int != tmp_long) {
174-
giterr_set(GITERR_INVALID, "failed to convert: '%s' is too large", nptr);
175+
int len = tmp_endptr - nptr;
176+
giterr_set(GITERR_INVALID, "failed to convert: '%.*s' is too large", len, nptr);
175177
return -1;
176178
}
177179

178180
*result = tmp_int;
181+
if (endptr)
182+
*endptr = tmp_endptr;
179183

180184
return error;
181185
}

tests/core/strtol.c

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,3 +75,10 @@ void test_core_strtol__buffer_length_truncates(void)
7575
cl_git_pass(git__strntol64(&i64, "11", 1, NULL, 10));
7676
cl_assert_equal_i(i64, 1);
7777
}
78+
79+
void test_core_strtol__error_message_cuts_off(void)
80+
{
81+
assert_l32_fails("2147483657foobar", 10);
82+
cl_assert(strstr(giterr_last()->message, "2147483657") != NULL);
83+
cl_assert(strstr(giterr_last()->message, "foobar") == NULL);
84+
}

0 commit comments

Comments
 (0)