Re: [PATCH 3/3] verify_path: consider dos drive prefix

[Johannes Sixt <j6t@kdbg.org>]

Am 27.05.2011 18:00, schrieb Erik Faye-Lund:
> If someone manage to create a repo with a 'C:' entry in the
> root-tree, files can be written outside of the working-dir. This
> opens up a can-of-worms of exploits.
> 
> Fix it by explicitly checking for a dos drive prefix when verifying
> a paht. While we're at it, make sure that paths beginning with '\' is
> considered absolute as well.

I think we do agree that the only way to avoid the security breach is to
check a path before it is used to write a file. In practice, it means to
disallow paths in the top-most level of the index that are two
characters long and are letter-colon.

IMHO, it is pointless to avoid that an evil path enters the repository,
because there are so many and a few more ways to create an evil repository.

> diff --git a/read-cache.c b/read-cache.c
> index f38471c..68faa51 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -753,11 +753,14 @@ int verify_path(const char *path)
>  {
>  	char c;
>  
> +	if (has_dos_drive_prefix(path))
> +		return 0;
> +

Isn't verify_path used to avoid that a bogus path enters the index? (I
don't know, I'm not familiar with this infrastructure.)

>  	goto inside;
>  	for (;;) {
>  		if (!c)
>  			return 1;
> -		if (c == '/') {
> +		if (is_dir_sep(c)) {
>  inside:

And if so, at this point, all backslashes should have been converted to
forward-slashes already. If not, then this would just paper over the
real bug.

>  			c = *path++;
>  			switch (c) {

-- Hannes