Git repos can be created outside the $GITHOME directory

[Rogdham]

This is a security-related bug

Problem & POC

The name of the repos can contain sequences such as ../, which allows the repositories to be stored outside the $GITHOME directory.

For example, the command git clone [email protected]:../../tmp/foo creates the repo in the directory /tmp/foo (with default configuration).

Also, one could create a repo named foo, then someone else a repo named foo/bar, which will completely hides the existence of the second repo. Also, replace bar for refs and you have an other error.

Solution

The solution is to filter the allowed repository name and/or to escape them.

The following code is to be changed:

parse_repo_from_ssh_command() {
  awk '{print $2}' | perl -pe 's/(?<!\\)'\''//g' | sed 's/\\'\''/'\''/g' | strip_root_slash
}

However, I don't have the perl knowledge to be sure to understand the code already, so I leave the fix to others.

Here is my suggestion:

• Make sure that there is no / in the repo name. Exit right away with an error message if there is.
• Also, it may be a good idea to check that the repo name does not start with a dot, so that a simple ls $GITHOME will show all repos.

Edit: Not allowing / in the repo name may be asking for too much (e.g. can not use user/something as repo names a la GitHub). Alternatively, we could replace / for some character (-? _? a space?), but this may lead to conflicts (e.g. user/something and user-something would be the same repo).

I will happily review commits aiming at fixing this flaw, provided they don't use any perl (as I don't have any knowledge in the area).

[Paperback]

For me, the git user cannot write outside their home directory so this fails with permission denied. I know it's best practise not to just hope for sane permissions particularly when dealing with security, but is there a sane use case where someone wants relative paths when pushing, for instance pushing configuration files into etc?

Instead of removing / from the repo name, perhaps it's better to just disallow relative paths by removing ..

Also that perl call has been replaced by sed.