From: Elijah Newren <newren@gmail.com>
Date: Mon, 20 Apr 2026 14:51:31 GMT
Subject: Re: [PATCH v2] diff: fix out-of-bounds reads and NULL deref in diffstat UTF-8 truncation
Message-ID: <CABPp-BHgnyS_SB6SX1dzAezfExomHts1t02+qr+duCPW6sk1nQ@mail.gmail.com>
In-Reply-To: <aeVqqsdq9B7GE9gS@lorenzo-VM>

On Sun, Apr 19, 2026 at 4:52 PM Lorenzo Pegorari
<lorenzo.pegorari2002@gmail.com> wrote:
>
> > +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?

Sorry, I did two things wrong -- I forgot to specify that the second
test only fails under ASan, and I simplified the test too much such
that it doesn't fail under ASan without the fixes (and simplified in
three wrong ways: not enough control characters, wrong kind of control
character, attempting to use hex control code to printf instead of
octal) and apparently forgot to re-check afterwards.  Using the
filename
    FNAME=$(printf "aaa-\302\237\302\237\302\237-aaa") &&
will trigger the out-of-bounds read under ASan before the fixes;
removing the final \302\237 will make it pass with or without the code
fixes.  I'll correct the patch and send in a new round.

Thanks for checking closely.