Re: [PATCH 6/6] config: reject parsing of files over INT_MAX

From: Jeff King <peff@peff.net>
Date: Apr 10, 2020, 22:15 UTC
Message-ID: <20200410221549.GA2078378@coredump.intra.peff.net>
In-Reply-To: <xmqqzhbjgfps.fsf@gitster.c.googlers.com>

On Fri, Apr 10, 2020 at 03:04:31PM -0700, Junio C Hamano wrote:

Show 15 quoted lines

> Jeff King <peff@peff.net> writes:
> 
> > So given the fact that these are insane cases which we have no need to
> > support, the weird behavior from feeding the results to printf even if
> > the code is careful, and the possibility of uncareful code introducing
> > its own integer truncation issues, let's just declare INT_MAX as a limit
> > for parsing config files.
> 
> Makes sense.
> 
> > +	if (c != EOF && ++cf->total_len > INT_MAX) {
> 
> Would this work correctly if size_t is uint?  Sure, as int-max would
> fit within it.  And of course if size_t is wider than uint, there is
> no problem in this comparison.

Good question, but yeah, I think it's right.

Another method would be to do:

  if (cf->total_len >= INT_MAX)

_before_ reading any character. We'd have to remember to increment total_len then (I suppose we could do it preemptively; as long as people don't try to read EOF from us over and over again it would never move again).

I also considered making the limit much lower than INT_MAX because really, who needs even a 1GB config file? :)

-Peff