git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Lorenzo Pegorari <lorenzo.pegorari2002@gmail.com>
Date
Apr 19, 2026, 23:52 UTC
Message-ID
<aeVqqsdq9B7GE9gS@lorenzo-VM>
In-Reply-To
<pull.2093.v2.git.1776465910538.gitgitgadget@gmail.com>
On Fri, Apr 17, 2026 at 10:45:10PM +0000, Elijah Newren via GitGitGadget wrote:
Show 41 quoted lines
> From: Elijah Newren <newren@gmail.com>
> 
> f85b49f3d4a (diff: improve scaling of filenames in diffstat to handle
> UTF-8 chars, 2026-01-16) 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 both issues by introducing utf8_ish_width(), a thin wrapper
> around utf8_width() that guarantees the pointer always advances and
> the returned width is never negative:
> 
>   - On invalid UTF-8 it restores the pointer, advances by one byte,
>     and returns width 1 (matching the strlen()-based fallback used
>     by utf8_strwidth()).
>   - On a control character it returns 0 (matching utf8_strnwidth()
>     which skips them).
> 
> Also add a "&& *name" guard to the while-loop condition so it
> terminates at end-of-string even when utf8_strwidth()'s strlen()
> fallback causes name_len to exceed the sum of per-character widths.
i> 
> Signed-off-by: Elijah Newren <newren@gmail.com>
Hi, thanks for CCing me and thanks for improving on my previous work.

All of these changes make a lot of sense, and indeed they fix issues that I didn't consider in f85b49f3d4a (diff: improve scaling of filenames in diffstat to handle UTF-8 chars, 2026-01-16).

[...]
Show 5 quoted lines
> diff --git a/t/t4052-stat-output.sh b/t/t4052-stat-output.sh
> index 7c749062e2..84c53c1a51 100755
> --- a/t/t4052-stat-output.sh
> +++ b/t/t4052-stat-output.sh
> @@ -445,4 +445,29 @@ test_expect_success 'diffstat where line_prefix contains ANSI escape codes is co
[...]
Show 15 quoted lines
>
> +test_expect_success FUNNYNAMES 'diffstat truncation with control chars does not crash' '
> +	FNAME=$(printf "aaa-\x01-aaa") &&
> +	git commit --allow-empty -m setup &&
> +	>$FNAME &&
> +	git add -- $FNAME &&
> +	git commit -m "add file with control char name" &&
> +	git -c core.quotepath=false diff --stat --stat-name-width=5 HEAD~1..HEAD >output &&
> +	test_grep "| 0" output &&
> +	rm -- $FNAME &&
> +	git rm -- $FNAME &&
> +	git commit -m "remove test file"
> +'
> +
>  test_done
The only thing that I don't quite understand is this second test.
From my tests, the previous code using:
```
[...]
while (name_len > len)
	name_len -= utf8_width((const char**)&name, NULL);
[...]
```

passes this second test just fine, while I believe it's supposed to fail.

Am I missing something?

Thanks, Lorenzo

Previous: Elijah Newren via GitGitGadgetNext: Elijah Newren
Message 6 of 9 in “diff: fix out-of-bounds reads and NULL deref in diffstat UTF-8 truncation”
  1. diff: fix out-of-bounds reads and NULL deref in diffstat UTF-8 truncationElijah Newren via GitGitGadget, Apr 17, 2026
  2. Junio C HamanoApr 17, 2026
  3. Elijah NewrenApr 17, 2026
  4. Junio C HamanoApr 17, 2026
  5. diff: fix out-of-bounds reads and NULL deref in diffstat UTF-8 truncationElijah Newren via GitGitGadget, Apr 17, 2026
  6. Lorenzo PegorariApr 19, 2026
  7. Elijah NewrenApr 20, 2026
  8. diff: fix out-of-bounds reads and NULL deref in diffstat UTF-8 truncationElijah Newren via GitGitGadget, Apr 20, 2026
  9. Junio C HamanoApr 20, 2026

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.