Re: [PATCH v3] diff: check range before dereferencing an array element
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Apr 29, 2025, 23:46 UTC
- Message-ID
- <xmqqy0vi350e.fsf@gitster.g>
- In-Reply-To
- <20250429233314.GA594987@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 11 quoted lines
> Ah, you're right. I was reading the offset check as "are we past the end > of string" (guided by CodeQL's complaint), and if that were the case the > logic would apply equally to all values we are checking. > > But that is not what is going on at all. The offset check is for "len - > 1", and so is "do not do this one CR match for the final character of > the string". And thus applying it elsewhere is wrong. > > And CodeQL's false positive is doubly wrong. We do not even need to say > "the string is NUL-terminated, so it is OK in this case to look past the > end-of-string". The check is not even a string bounds check at all.
Exactly.
That is what makes it hard to give a reasonable explanation in the log message, which I thought that Dscho did a much better job in this iteration.
Thanks.