Re: [PATCH] mailinfo: fix out-of-bounds memory reads in unquote_quoted_pair()
- From
Jeff King <peff@peff.net>
- Date
- Dec 14, 2023, 21:40 UTC
- Message-ID
- <20231214214057.GA2297853@coredump.intra.peff.net>
- In-Reply-To
- <xmqqy1dynofo.fsf@gitster.g>
On Wed, Dec 13, 2023 at 06:54:03AM -0800, Junio C Hamano wrote:
Show 14 quoted lines
> 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