On Fri, May 31, 2024 at 08:05:03AM -0700, Junio C Hamano wrote: > Jeff King <peff@xxxxxxxx> 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
