CVE-2012-4506: Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal') (CWE-22)
value: Directory traversal vulnerability in gitolite 3.x before 3.1, when wild card repositories and a pattern matching "../" are enabled, allows remote authenticated users to create arbitrary repositories and possibly perform other actions via a .. (dot dot) in a repository name.
CWE description: The product uses external input to construct a pathname that is intended to identify a file or directory that is located underneath a restricted parent directory, but the product does not properly neutralize special elements within the pathname that can cause the pathname to resolve to a location that is outside of the restricted directory.
Published: 2012-10-22T23:55Z
Repository: https://github.com/sitaramc/gitolite
Language: Other
Files changed: src/gitolite-shell, src/lib/Gitolite/Conf/Load.pm, t/0-me-first.t
Commit date: 2012-10-05 12:28:20 +0530
Commit message: (security) fix bug in pattern to detect path traversal

while we're about it, add the same check to some of the internal
routines, so that commands can also be protected.

finally, just to make sure we don't lose it again in some other fashion,
add a few tests for path traversal...

Diff: diff --git a/src/gitolite-shell b/src/gitolite-shell
index 5d48cc9..4bbae48 100755
--- a/src/gitolite-shell
+++ b/src/gitolite-shell
@@ -166,11 +166,11 @@ sub parse_soc {
 
 sub sanity {
     my $repo = shift;
     _die "'$repo' contains bad characters" if $repo !~ $REPONAME_PATT;
     _die "'$repo' ends with a '/'"         if $repo =~ m(/$);
-    _die "'$repo' contains '..'"           if $repo =~ m(\.\.$);
+    _die "'$repo' contains '..'"           if $repo =~ m(\.\.);
 }
 
 # ----------------------------------------------------------------------
 # helper functions for "in_http"
 
diff --git a/src/lib/Gitolite/Conf/Load.pm b/src/lib/Gitolite/Conf/Load.pm
index 4abfa90..0f76908 100644
--- a/src/lib/Gitolite/Conf/Load.pm
+++ b/src/lib/Gitolite/Conf/Load.pm
@@ -65,12 +65,13 @@ my $last_repo = '';
     }
 }
 
 sub access {
     my ( $repo, $user, $aa, $ref ) = @_;
-    _die "invalid repo '$repo'" if not( $repo and $repo =~ $REPOPATT_PATT );
     _die "invalid user '$user'" if not( $user and $user =~ $USERNAME_PATT );
+    sanity($repo);
+
     my $deny_rules = option( $repo, 'deny-rules' );
     load($repo);
 
     # sanity check the only piece the user can control
     _die "invalid characters in ref or filename: '$ref'\n" unless $ref =~ $REF_OR_FILENAME_PATT;
@@ -173,12 +174,22 @@ sub option {
     my $ret = git_config( $repo, "^\Q$option\E\$" );
     return '' unless %$ret;
     return $ret->{$option};
 }
 
+sub sanity {
+    my $repo = shift;
+
+    _die "invalid repo '$repo'" if not( $repo and $repo =~ $REPOPATT_PATT );
+    _die "'$repo' ends with a '/'" if $repo =~ m(/$);
+    _die "'$repo' contains '..'" if $repo =~ $REPONAME_PATT and $repo =~ m(\.\.);
+}
+
 sub repo_missing {
     my $repo = shift;
+    sanity($repo);
+
     return not -d "$rc{GL_REPO_BASE}/$repo.git";
 }
 
 # ----------------------------------------------------------------------
 
@@ -398,10 +409,12 @@ sub generic_name {
     return $base2;
 }
 
 sub creator {
     my $repo = shift;
+    sanity($repo);
+
     return ( $ENV{GL_USER} || '' ) if repo_missing($repo);
     my $f       = "$rc{GL_REPO_BASE}/$repo.git/gl-creator";
     my $creator = '';
     chomp( $creator = slurp($f) ) if -f $f;
     return $creator;
diff --git a/t/0-me-first.t b/t/0-me-first.t
index dc8916b..22102ef 100755
--- a/t/0-me-first.t
+++ b/t/0-me-first.t
@@ -4,14 +4,16 @@ use warnings;
 
 # this is hardcoded; change it if needed
 use lib "src/lib";
 use Gitolite::Test;
 
+my $rb = `gitolite query-rc -n GL_REPO_BASE`;
+
 # initial smoke tests
 # ----------------------------------------------------------------------
 
-try "plan 65";
+try "plan 73";
 
 # basic push admin repo
 confreset;confadd '
     repo aa
         RW+     =   u1
@@ -73,6 +75,21 @@ try "
     glt ls-remote u4 file:///bb;    !ok;    /DENIED by fallthru/
     glt ls-remote u4 file:///cc/1;  ok;     /Initialized empty/
     glt ls-remote u5 file:///cc/1;  ok;     perl s/TRACE.*//g; !/\\S/
     glt ls-remote u5 file:///cc/2;  !ok;    /DENIED by fallthru/
     glt ls-remote u6 file:///cc/2;  !ok;    /DENIED by fallthru/
+
+    # command
+    glt perms u4 -c cc/bar/baz/frob + READERS u2;
+                                    ok;     /Initialized empty .*cc/bar/baz/frob.git/
+
+    # path traversal
+    glt ls-remote u4 file:///cc/dd/../ee
+                                    !ok;    /FATAL: 'cc/dd/\\.\\./ee' contains '\\.\\.'/
+    glt ls-remote u5 file:///cc/../../../../../..$rb/gitolite-admin
+                                    !ok;    /FATAL: 'cc/../../../../../..$rb/gitolite-admin' contains '\\.\\.'/
+
+    glt perms u4 -c cc/bar/baz/../frob + READERS u2
+                                    !ok;    /FATAL: 'cc/bar/baz/\\.\\./frob' contains '\\.\\.'/
+
+
 ";

Vulnerable code:     _die "'$repo' contains '..'"           if $repo =~ m(\.\.$);
    _die "invalid repo '$repo'" if not( $repo and $repo =~ $REPOPATT_PATT );
try "plan 65";
Fixed code:     _die "'$repo' contains '..'"           if $repo =~ m(\.\.);
    sanity($repo);

sub sanity {
    my $repo = shift;

    _die "invalid repo '$repo'" if not( $repo and $repo =~ $REPOPATT_PATT );
    _die "'$repo' ends with a '/'" if $repo =~ m(/$);
    _die "'$repo' contains '..'" if $repo =~ $REPONAME_PATT and $repo =~ m(\.\.);
}

    sanity($repo);

    sanity($repo);

my $rb = `gitolite query-rc -n GL_REPO_BASE`;

try "plan 73";

    # command
    glt perms u4 -c cc/bar/baz/frob + READERS u2;
                                    ok;     /Initialized empty .*cc/bar/baz/frob.git/

    # path traversal
    glt ls-remote u4 file:///cc/dd/../ee
                                    !ok;    /FATAL: 'cc/dd/\\.\\./ee' contains '\\.\\.'/
    glt ls-remote u5 file:///cc/../../../../../..$rb/gitolite-admin
                                    !ok;    /FATAL: 'cc/../../../../../..$rb/gitolite-admin' contains '\\.\\.'/

    glt perms u4 -c cc/bar/baz/../frob + READERS u2
                                    !ok;    /FATAL: 'cc/bar/baz/\\.\\./frob' contains '\\.\\.'/