Re: [PATCH] Fix buffer overflow in git-grep

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Jul 16, 2008, 10:35 UTC
Message-ID: <alpine.DEB.1.00.0807161232110.8503@eeepc-johanness>
In-Reply-To: <1216203345-18233-1-git-send-email-dpotapov@gmail.com>

Hi,

On Wed, 16 Jul 2008, Dmitry Potapov wrote:

> If PATH_MAX on your system is smaller than any path stored in the git 
> repository, that can cause memory corruption inside of the grep_tree 
> function used by git-grep.

Let me guess: Windows? *giggles*

Show 13 quoted lines

> diff --git a/builtin-grep.c b/builtin-grep.c
> index ef29910..530a53d 100644
> --- a/builtin-grep.c
> +++ b/builtin-grep.c
> @@ -441,14 +441,17 @@ static int grep_tree(struct grep_opt *opt, const char **paths,
>  	len = strlen(path_buf);
>  
>  	while (tree_entry(tree, &entry)) {
> -		strcpy(path_buf + len, entry.path);
> +		int te_len = tree_entry_len(entry.path, entry.sha1);
> +		if (len + te_len >= PATH_MAX + tn_len)
> +			die ("path too long: %s", path_buf+tn_len);
> +		memcpy(path_buf + len, entry.path, te_len);

That is brutal. Does grep_tree() not work on tree objects in memory? In that case, you prevent the user from grepping, only because she is on a suboptimal platform, _even if_ even that platform could cope with it.

It's not like the path is ever used to access a file, right?

Maybe you should convert the path_buf to a strbuf instead.

Ciao, Dscho