Re: [PATCH] Git segmentation faults if submodule path is empty.

[Jens Lehmann <Jens.Lehmann@web.de>]

Am 16.08.2013 03:51, schrieb Jharrod LaFon:
> Git fails due to a segmentation fault if a submodule path is empty.
> Here is an example .gitmodules that will cause a segmentation fault:
> [submodule "foo-module"]
>     path
>     url = http://host/repo.git
> $ git status
> Segmentation fault (core dumped)
> 
> This occurs because in the function parse_submodule_config_option, the
> variable 'value' is assumed not to be null, and when passed as an
> argument to xstrdup a segmentation fault occurs if it is indeed null.
> This is the case when using the .gitmodules example above.

Thanks for digging this up and describing it in a way that makes it
easy to reproduce and understand.

> This patch addresses the issue by returning from the function if
> 'value' is null before the call to xstrdup is made.

Hmm, I'm not sure silently ignoring the misconfiguration is the best
way to go. A submodule config having a path setting without a value
is broken (while a submodule setting without a subsection configures
something else, so the "|| !name" below is fine). So I believe we
should complain to the user when "value" is NULL.

On the other hand this should only happen for the three options we do
parse, as some users (e.g. git-submodule.sh) use other configurations
for which a missing value may be fine. Please see the "lacks value"
errors in read_merge_config() of ll-merge.c for an example of how to
deal with that.

And looking at other users of parse_config_key() I suspect there will
be other configuration options showing the same problem ...

> Signed-off-by: Jharrod LaFon <jlafon <at> eyesopen.com>
> ---
>  submodule.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/submodule.c b/submodule.c
> index 1821a5b..880f21b 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -130,7 +130,7 @@ int parse_submodule_config_option(const char *var, const char *value)
>  	const char *name, *key;
>  	int namelen;
>  
> -	if (parse_config_key(var, "submodule", &name, &namelen, &key) < 0 || !name)
> +	if (parse_config_key(var, "submodule", &name, &namelen, &key) < 0 || !name || !value)
>  		return 0;
>  
>  	if (!strcmp(key, "path")) {
>