tighten up ref/file names (warning: possible backward compat breakage)

The backward compat breakage is for people who already have all kinds of
arbitrary characters in filenames *and* use `NAME/` rules.  See the doc
change in this commit for details and mitigation.  See this link for
background:

    http://groups.google.com/group/gitolite/browse_thread/thread/8dc5242052b16d0f

Thanks to Dan Carpenter for the audit.
This commit is contained in:
Sitaram Chamarty 2011-10-01 07:32:29 +05:30
parent 871ed281cc
commit a07e0d6b5c
5 changed files with 124 additions and 1 deletions

View file

@ -56,6 +56,7 @@ $SVNSERVE = "";
# $GL_ADC_PATH = ""; # $GL_ADC_PATH = "";
# $GL_GET_MEMBERSHIPS_PGM = "/usr/local/bin/expand-ldap-user-to-groups" # $GL_GET_MEMBERSHIPS_PGM = "/usr/local/bin/expand-ldap-user-to-groups"
# $GL_HTTP_ANON_USER = "mob"; # $GL_HTTP_ANON_USER = "mob";
# $GL_REF_OR_FILENAME_PATT=qr(^[0-9a-zA-Z][0-9a-zA-Z._\@/+ :,-]*$);
# ------------------------------------------------------------------------------ # ------------------------------------------------------------------------------
# less used/changed variables # less used/changed variables

View file

@ -260,6 +260,21 @@ on feedback from my users to find or fix issues.
gitolite that unauthenticated HTTP users are actually authenticated as gitolite that unauthenticated HTTP users are actually authenticated as
this user. this user.
* `$GL_REF_OR_FILENAME_PATT`, string
Set of allowed characters in refnames (and, if you have `NAME/` rules, in
filenames as well). The default pattern is almost the same as
`$REPONAME_PATT` with some additions.
Although the current code is not at risk in any way even if we let in
names containing strings like `$(command)`, and although I intend to make
sure things stay that way, it's probably a good idea to trap weird
filenames early. Just to be safe.
You ought to be able to loosen the pattern by adding other characters to
it, if you really need to. If you do, at least avoid backquotes and the
dollar sign!
<a name="_less_used_changed_variables"></a> <a name="_less_used_changed_variables"></a>
### less used/changed variables ### less used/changed variables

View file

@ -213,6 +213,10 @@ sub check_ref {
# NOTE: the function DIES when access is denied, unless arg 5 is true # NOTE: the function DIES when access is denied, unless arg 5 is true
my ($allowed_refs, $repo, $ref, $perm, $dry_run) = @_; my ($allowed_refs, $repo, $ref, $perm, $dry_run) = @_;
# sanity check the ref
die "invalid characters in ref or filename: $ref\n" unless $ref =~ $GL_REF_OR_FILENAME_PATT;
my @allowed_refs = sort { $a->[0] <=> $b->[0] } @{$allowed_refs}; my @allowed_refs = sort { $a->[0] <=> $b->[0] } @{$allowed_refs};
for my $ar (@allowed_refs) { for my $ar (@allowed_refs) {
my $refex = $ar->[1]; my $refex = $ar->[1];

View file

@ -9,7 +9,7 @@ use Exporter 'import';
@EXPORT = qw( @EXPORT = qw(
$ABRT $WARN $ABRT $WARN
$R_COMMANDS $W_COMMANDS $R_COMMANDS $W_COMMANDS
$REPONAME_PATT $USERNAME_PATT $REPOPATT_PATT $REPONAME_PATT $USERNAME_PATT $REPOPATT_PATT $GL_REF_OR_FILENAME_PATT
$ADC_CMD_ARGS_PATT $ADC_CMD_ARGS_PATT
$BIG_INFO_CAP $BIG_INFO_CAP
$current_data_version $current_data_version
@ -48,6 +48,8 @@ $REPONAME_PATT=qr(^\@?[0-9a-zA-Z][0-9a-zA-Z._\@/+-]*$);
$USERNAME_PATT=qr(^\@?[0-9a-zA-Z][0-9a-zA-Z._\@+-]*$); $USERNAME_PATT=qr(^\@?[0-9a-zA-Z][0-9a-zA-Z._\@+-]*$);
# same as REPONAME, but used for wildcard repos, allows some common regex metas # same as REPONAME, but used for wildcard repos, allows some common regex metas
$REPOPATT_PATT=qr(^\@?[0-9a-zA-Z[][\\^.$|()[\]*+?{}0-9a-zA-Z._\@/,-]*$); $REPOPATT_PATT=qr(^\@?[0-9a-zA-Z[][\\^.$|()[\]*+?{}0-9a-zA-Z._\@/,-]*$);
# pattern for refnames pushed or names of files changed
$GL_REF_OR_FILENAME_PATT=qr(^[0-9a-zA-Z][0-9a-zA-Z._\@/+ :,-]*$);
# ADC commands and arguments must match this pattern # ADC commands and arguments must match this pattern
$ADC_CMD_ARGS_PATT=qr(^[0-9a-zA-Z._\@/+:-]*$); $ADC_CMD_ARGS_PATT=qr(^[0-9a-zA-Z._\@/+:-]*$);

101
t/t69-bad-ref-file-names Normal file
View file

@ -0,0 +1,101 @@
cd $TESTDIR
$TESTDIR/rollback || die "rollback failed"
# ----------
# vim: syn=sh:
# ----------
name "setup"
echo "
repo aa
RW+ = @all
" | ugc
# reasonably complex setup; we'll do everything from one repo though
cd ~/td
rm -rf aa
runlocal git clone u1:aa
cd ~/td/aa
mdc file-1
name "INTERNAL"
runlocal git push origin HEAD
expect "To u1:aa"
expect_push_ok "\* \[new branch\] HEAD -> master"
name "push file aa,bb ok"
mdc aa,bb
runlocal git push origin HEAD
expect "To u1:aa"
expect_push_ok "HEAD -> master"
name "push file aa=bb ok"
mdc aa=bb
runlocal git push origin HEAD
expect "To u1:aa"
expect_push_ok "HEAD -> master"
name "push to branch dd,ee ok"
runlocal git push origin master:dd,ee
expect "To u1:aa"
expect_push_ok "\* \[new branch\] master -> dd,ee"
name "push to branch dd=ee fail"
runlocal git push origin master:dd=ee
expect "remote: invalid characters in ref or filename: refs/heads/dd=ee"
expect "remote: error: hook declined to update refs/heads/dd=ee"
expect "\[remote rejected\] master -> dd=ee (hook declined)"
expect "error: failed to push some refs to 'u1:aa'"
name "INTERNAL"
cd $TESTDIR
$TESTDIR/rollback || die "rollback failed"
# ----------
name "setup"
echo "
repo aa
RW+ = @all
RW+ NAME/ = @all
" | ugc -r
# reasonably complex setup; we'll do everything from one repo though
cd ~/td
rm -rf aa
runlocal git clone u1:aa
cd ~/td/aa
mdc file-1
name "INTERNAL"
runlocal git push origin HEAD
expect "To u1:aa"
expect_push_ok "\* \[new branch\] HEAD -> master"
name "push file aa,bb ok"
mdc aa,bb
runlocal git push origin HEAD
expect "To u1:aa"
expect_push_ok "HEAD -> master"
name "push file aa=bb fail"
mdc aa=bb
runlocal git push origin HEAD
expect "To u1:aa"
expect "remote: invalid characters in ref or filename: NAME/aa=bb"
expect "remote: error: hook declined to update refs/heads/master"
expect "\[remote rejected\] HEAD -> master (hook declined)"
expect "error: failed to push some refs to 'u1:aa'"
name "push to branch dd,ee ok"
runlocal git reset --hard HEAD^
mdc some-file
runlocal git push origin master:dd,ee
expect "To u1:aa"
expect_push_ok "\* \[new branch\] master -> dd,ee"
name "push to branch dd=ee fail"
runlocal git push origin master:dd=ee
expect "remote: invalid characters in ref or filename: refs/heads/dd=ee"
expect "remote: error: hook declined to update refs/heads/dd=ee"
expect "\[remote rejected\] master -> dd=ee (hook declined)"
expect "error: failed to push some refs to 'u1:aa'"
name "INTERNAL"