Skip to content

Commit e584e2a

Browse files
committed
diff: fix out-of-bounds reads and NULL deref in diffstat UTF-8 truncation
f85b49f (diff: improve scaling of filenames in diffstat to handle UTF-8 chars, 2024-10-27) introduced a loop in show_stats() that calls utf8_width() repeatedly to skip leading characters until the displayed width fits. However, utf8_width() can return problematic values: - For invalid UTF-8 sequences, pick_one_utf8_char() sets the name pointer to NULL and utf8_width() returns 0. Since name_len does not change, the loop iterates once more and pick_one_utf8_char() dereferences the NULL pointer, crashing. - For control characters, utf8_width() returns -1, so name_len grows when it is expected to shrink. This can cause the loop to consume more characters than the string contains, reading past the trailing NUL. By default, fill_print_name() will C-quotes filenames which escapes control characters and invalid bytes to printable text. That avoids this bug from being triggered; however, with core.quotePath=false, raw bytes can reach this code. Add tests exercising both failure modes with core.quotePath=false and a narrow --stat-name-width to force truncation: one with a bare 0xC0 byte (invalid UTF-8 lead byte, triggers NULL deref) and one with a 0x01 byte (control character, causes the loop to read past the end of the string). Fix the bug by: - Adding a *name check to terminate the loop at end-of-string - Detecting the NULL pointer from invalid UTF-8 and falling back to showing the full untruncated name - Breaking on negative width (control characters) Signed-off-by: Elijah Newren <newren@gmail.com>
1 parent 9f223ef commit e584e2a

File tree

2 files changed

+37
-2
lines changed

2 files changed

+37
-2
lines changed

diff.c

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3093,8 +3093,17 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
30933093
if (len < 0)
30943094
len = 0;
30953095

3096-
while (name_len > len)
3097-
name_len -= utf8_width((const char**)&name, NULL);
3096+
while (name_len > len && *name) {
3097+
int w = utf8_width((const char **)&name, NULL);
3098+
if (!name) { /* Invalid UTF-8 */
3099+
name = file->print_name;
3100+
name_len = utf8_strwidth(name);
3101+
break;
3102+
}
3103+
if (w < 0) /* control character */
3104+
break;
3105+
name_len -= w;
3106+
}
30983107

30993108
slash = strchr(name, '/');
31003109
if (slash)

t/t4052-stat-output.sh

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -445,4 +445,30 @@ test_expect_success 'diffstat where line_prefix contains ANSI escape codes is co
445445
test_grep "<RED>|<RESET> ${FILENAME_TRIMMED} | 0" out
446446
'
447447

448+
test_expect_success FUNNYNAMES 'diffstat truncation with invalid UTF-8 does not crash' '
449+
FNAME=$(printf "aaa-\xc0-aaa") &&
450+
git commit --allow-empty -m setup &&
451+
>$FNAME &&
452+
git add -- $FNAME &&
453+
git commit -m "add file with invalid UTF-8 name" &&
454+
git -c core.quotepath=false diff --stat --stat-name-width=5 HEAD~1..HEAD >output &&
455+
test_grep "| 0" output &&
456+
rm -- $FNAME &&
457+
git rm -- $FNAME &&
458+
git commit -m "remove test file"
459+
'
460+
461+
test_expect_success FUNNYNAMES 'diffstat truncation with control chars does not crash' '
462+
FNAME=$(printf "aaa-\x01-aaa") &&
463+
git commit --allow-empty -m setup &&
464+
>$FNAME &&
465+
git add -- $FNAME &&
466+
git commit -m "add file with control char name" &&
467+
git -c core.quotepath=false diff --stat --stat-name-width=5 HEAD~1..HEAD >output &&
468+
test_grep "| 0" output &&
469+
rm -- $FNAME &&
470+
git rm -- $FNAME &&
471+
git commit -m "remove test file"
472+
'
473+
448474
test_done

0 commit comments

Comments
 (0)