Re: [PATCH] archive-tar: fix sanity check in config parsing

From: Jeff King <peff@peff.net>
Date: Jan 13, 2013, 20:00 UTC
Message-ID: <20130113200044.GA3979@sigill.intra.peff.net>
In-Reply-To: <50F2F1E9.1040700@lsrfire.ath.cx>

On Sun, Jan 13, 2013 at 06:42:01PM +0100, René Scharfe wrote:

Show 10 quoted lines

> When parsing these config variable names, we currently check that
> the second dot is found nine characters into the name, disallowing
> filter names with a length of five characters.  Additionally,
> git archive crashes when the second dot is omitted:
> 
> 	$ ./git -c tar.foo=bar archive HEAD >/dev/null
> 	fatal: Data too large to fit into virtual memory space.
> 
> Instead we should check if the second dot exists at all, or if
> we only found the first one.

Eek. Thanks for finding it. Your fix is obviously correct.

Show 9 quoted lines

> --- a/archive-tar.c
> +++ b/archive-tar.c
> @@ -335,7 +335,7 @@ static int tar_filter_config(const char *var, const char *value, void *data)
>  	if (prefixcmp(var, "tar."))
>  		return 0;
>  	dot = strrchr(var, '.');
> -	if (dot == var + 9)
> +	if (dot == var + 3)
>  		return 0;

For the curious, the original version of the patch[1] read:

+       if (prefixcmp(var, "tarfilter."))
+               return 0;
+       dot = strrchr(var, '.');
+       if (dot == var + 9)
+               return 0;

and when I shortened the config section to "tar" in a re-roll of the series, I missed the corresponding change to the offset.

-Peff

[1] http://thread.gmane.org/gmane.comp.version-control.git/175785/focus=175858