deac/gitolite
1
0
Fork 0
Code Issues Pull requests Releases Wiki Activity

(security) fix bug in pattern to detect path traversal

Browse source 
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...
This commit is contained in:
Sitaram Chamarty 2012-10-05 07:19:59 +05:30
parent 0d371ac957
commit f636ce3ba3
3 changed files with 33 additions and 3 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
src/gitolite-shell
View file

@ -168,7 +168,7 @@ sub sanity {
my $repo = shift; my $repo = shift;
_die "'$repo' contains bad characters" if $repo !~ $REPONAME_PATT; _die "'$repo' contains bad characters" if $repo !~ $REPONAME_PATT;
_die "'$repo' ends with a '/'" if $repo =~ m(/$); _die "'$repo' ends with a '/'" if $repo =~ m(/$);
_die "'$repo' contains '..'" if $repo =~ m(\.\.$); _die "'$repo' contains '..'" if $repo =~ m(\.\.);
} }
# ---------------------------------------------------------------------- # ----------------------------------------------------------------------

15
src/lib/Gitolite/Conf/Load.pm
View file

@ -67,8 +67,9 @@ my $last_repo = '';
sub access { sub access {
my ( $repo, $user, $aa, $ref ) = @_; 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 ); _die "invalid user '$user'" if not( $user and $user =~ $USERNAME_PATT );
sanity($repo);
my $deny_rules = option( $repo, 'deny-rules' ); my $deny_rules = option( $repo, 'deny-rules' );
load($repo); load($repo);
@ -175,8 +176,18 @@ sub option {
return $ret->{$option}; 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 { sub repo_missing {
my $repo = shift; my $repo = shift;
sanity($repo);
return not -d "$rc{GL_REPO_BASE}/$repo.git"; return not -d "$rc{GL_REPO_BASE}/$repo.git";
} }
@ -400,6 +411,8 @@ sub generic_name {
sub creator { sub creator {
my $repo = shift; my $repo = shift;
sanity($repo);
return ( $ENV{GL_USER} || '' ) if repo_missing($repo); return ( $ENV{GL_USER} || '' ) if repo_missing($repo);
my $f = "$rc{GL_REPO_BASE}/$repo.git/gl-creator"; my $f = "$rc{GL_REPO_BASE}/$repo.git/gl-creator";
my $creator = ''; my $creator = '';

19
t/0-me-first.t
View file

@ -6,10 +6,12 @@ use warnings;
use lib "src/lib"; use lib "src/lib";
use Gitolite::Test; use Gitolite::Test;
my $rb = `gitolite query-rc -n GL_REPO_BASE`;
# initial smoke tests # initial smoke tests
# ---------------------------------------------------------------------- # ----------------------------------------------------------------------
try "plan 65"; try "plan 73";
# basic push admin repo # basic push admin repo
confreset;confadd ' confreset;confadd '
@ -75,4 +77,19 @@ try "
glt ls-remote u5 file:///cc/1; ok; perl s/TRACE.*//g; !/\\S/ 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 u5 file:///cc/2; !ok; /DENIED by fallthru/
glt ls-remote u6 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 '\\.\\.'/
"; ";