Re: [PATCH] mailinfo: fix out-of-bounds memory reads in unquote_quoted_pair()

[Jeff King <peff@peff.net>]

On Wed, Dec 13, 2023 at 06:54:03AM -0800, Junio C Hamano wrote:

> I actually had trouble with the proposed update, and wondered if
> 
> -	while ((c = *in++) != 0) {
> +	while ((c = *in)) {
> +		in++;
> 
> is easier to follow, but then was hit by the possibility that the
> same "we have incremented 'in' a bit too early" may exist if such
> a loop wants to use 'in' in its body.  Wouldn't it mean that
> 
> -	while ((c = *in++) != 0) {
> +	for (; c = *in; in++) {
> 
> would be even a better rewrite?

No, the "for" loop wouldn't work, because the loop body actually depends
on "in" having already been incremented. If we find the end of the
comment or quoted string, we return "in", and the caller is expecting it
to have moved past the closing quote. So that would have to become
"return in+1".

IOW, the issue is that the normal end-of-quote parsing and hitting
end-of-string are fundamentally different. So we either need to
differentiate the returns (either with "+1" on one, or "-1" on the
other). Or we need to choose to increment "in" based on which we found
(which is what my patch does).

-Peff