Re: [PATCH] dir.c: skip .gitignore, etc larger than INT_MAX

From: Jeff King <peff@peff.net>
Date: Jun 4, 2024, 10:39 UTC
Message-ID: <20240604103947.GC1781455@coredump.intra.peff.net>
In-Reply-To: <xmqqh6eehvj4.fsf@gitster.g>

On Fri, May 31, 2024 at 08:05:03AM -0700, Junio C Hamano wrote:

Show 10 quoted lines

> Jeff King <peff@peff.net> writes:
> 
> > The "int" in question is in add_patterns_from_buffer(), so we could
> > catch it there. But by putting the checks in its two callers, we can
> > produce more useful error messages.
> 
> Nice trick.  I wondered if we want INT_MAX/2 or something even
> lower, but because once these things are read, we only parse the
> contents so allowing up to INT_MAX is fine.  It is not like we read
> this and that and concatenate them into a larger buffer.

Yeah, I think all of the individual string lengths would be fine. It _might_ be possible to overflow an array of pointers or similar, though. E.g., if you have "a\n" repeated INT_MAX times, then you have INT_MAX/2 entries. An array of pointers-to-structs, assuming 8-byte pointers, would then need INT_MAX*4 bytes. And indeed, I think we stuff these into a pattern_list which uses ints.

In practice I think the call to ALLOC_GROW() would catch that as the overflowed value turns into a large size_t. But it probably would be safer to use a much smaller limit.

I was hoping to avoid making up an arbitrary number. But your question about gitattributes reminded me that we already did something similar in 3c50032ff5 (attr: ignore overly large gitattributes files, 2022-12-01). There it's a hard-coded 100MB limit (without even a config option).

Maybe we should just do the same here?

-Peff