Re: [PATCH] diff: fix out-of-bounds reads and NULL deref in diffstat UTF-8 truncation

[Junio C Hamano <gitster@pobox.com>]

"Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Elijah Newren <newren@gmail.com>
>
> f85b49f3d4a (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.

A tangent, but I get a datestamp for the same f85b49f3 (diff:
improve scaling of filenames in diffstat to handle UTF-8 chars,
2026-01-16) that is different from what you showed above.  Did you
find a bug in "git show -s --pretty=reference"?

> diff --git a/diff.c b/diff.c
> index 397e38b41c..7b27241733 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -3093,8 +3093,17 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
>  			if (len < 0)
>  				len = 0;
>  
> -			while (name_len > len)
> -				name_len -= utf8_width((const char**)&name, NULL);
> +			while (name_len > len && *name) {



> +				int w = utf8_width((const char **)&name, NULL);
> +				if (!name) { /* Invalid UTF-8 */
> +					name = file->print_name;
> +					name_len = utf8_strwidth(name);
> +					break;
> +				}

IOW, we punt on "scaling" and instead use the full string?  I was
wondering if we can punt on only this segment by replacing this
segment with just "..." and resync at the next slash.

> +				if (w < 0)  /* control character */
> +					break;

When we have a control characer, we instead chomp immediately before
that byte, which sounds good.  But then wouldn't the loop that found
an Invalid UTF-8 sequence in the middle of a name want to do the
same, i.e., take the good bits found so far and chomp at the broken
byte?

> +				name_len -= w;
> +			}
>  
>  			slash = strchr(name, '/');
>  			if (slash)

Thanks.