fix ref-create permission bug on wild repos

TLDR: users are able to *create* new refs that they are not supposed to.
Upgrading gitolite is not mandatory; there is a workaround within the conf
syntax itself (see below).

The bug shows up if the following four conditions are satisfied:

    repo foo/..*                    # 1 (see below)
        C       =   @all            # 2
        RW+CD   =   CREATOR         # 3
        RW      =   special_user    # 4

1.  must be a wild repo
2.  the create-repo line must allow "special_user(s)"
3.  there is at least one rule containing RW.*C in the repo
4.  the intent is that special user(s) can ff-push but cannot *create* refs
    since they have only RW; for a refresher on this, see
    http://gitolite.com/gitolite/conf.html#write-types

In such cases, the restriction on creating a ref is ignored.  (Only creation
is affected; the bug does not affect delete, rewind permissions).

In general, this is a relatively minor bug.  No data is destroyed, and no data
is leaked.  Some cleanup of useless refs may be required if someone has
(ab)used this.  At worst this could be a potential DOS, but I have never
considered DOS to be a valid concern when it requires *authorised* users.

If you have such a conf, and cannot immediately upgrade, there is a
workaround:

    repo foo/..*
        RW+CD   =   CREATOR
        RW      =   special_user

        -       =   special_user    # add a deny rule for the special user(s)
        C       =   @all            # move the repo-create perm to the end,
                                    # *after* the deny rule above

(Thanks to hseg on #gitolite for catching this and being tenacious about it!
At first, I was arrogant enough to reject the idea that such a bug could
exist; it took me a bit to get the right conditions in place and see the
problem!)

2 files changed, 57 insertions, 2 deletions

diff --git a/src/lib/Gitolite/Conf/Load.pm b/src/lib/Gitolite/Conf/Load.pm
index 88d1612..7728a5a 100644
--- a/src/lib/Gitolite/Conf/Load.pm
+++ b/src/lib/Gitolite/Conf/Load.pm
@@ -123,9 +123,21 @@ sub access {
         trace( 2, "DENIED by $refex" ) if $perm eq '-';
         return "$aa $safe_ref $repo $user DENIED by $refex" if $perm eq '-';
 
-        # $perm can be RW\+?(C|D|CD|DC)?M?.  $aa can be W, +, C or D, or
-        # any of these followed by "M".
+        # For repo creation, perm will be C and aa will be "^C".  For branch
+        # access, $perm can be RW\+?(C|D|CD|DC)?M?, and $aa can be W, +, C or
+        # D, or any of these followed by "M".
+
+        # We need to turn $aa into a regex that can match a suitable $perm.
+        # This is trivially true for "^C", "W" and "D", but the others (+, C,
+        # M) need some tweaking.
+
+        # first, quote the '+':
         ( my $aaq = $aa ) =~ s/\+/\\+/;
+        # if aa is just "C", the user is trying to create a *branch* (not a
+        # *repo*), so let's make the pattern clearer to reflect that.
+        $aaq = "RW.*C" if $aaq eq "C";
+        # if the aa is, say "WM", make this "W.*M" because the perm could be
+        # 'RW+M', 'RW+CDM' etc, and they are all valid:
         $aaq =~ s/M/.*M/;
 
         $rc{RULE_TRACE} .= "A";
diff --git a/t/C-vs-C.t b/t/C-vs-C.t
new file mode 100644
index 0000000..fee5cc4
--- /dev/null
+++ b/t/C-vs-C.t
@@ -0,0 +1,43 @@
+#!/usr/bin/perl
+use strict;
+use warnings;
+
+# the commit message in which this test is introduced should have details, but
+# briefly, this test makes sure that access() does not get confused by
+# repo-create permissions being allowed, when looking for branch-create
+# permissions.
+
+# this is hardcoded; change it if needed
+use lib "src/lib";
+use Gitolite::Test;
+
+# branch permissions test
+# ----------------------------------------------------------------------
+
+try "plan 25";
+
+confreset;confadd '
+    repo foo/..*
+        C       =   @all
+        RW+CD   =   CREATOR
+        RW      =   u2
+
+';
+
+try "ADMIN_PUSH set1; !/FATAL/" or die text();
+
+try "
+    cd ..;                          ok
+    glt clone u1 file:///foo/aa;    ok
+    cd aa;                          ok
+    tc l-1;                         ok;     /master/
+    glt push u1 origin master:m1;   ok;     /To file:///foo/aa/
+                                            /\\* \\[new branch\\]      master -> m1/
+
+    tc l-2;                         ok;     /master/
+    glt push u2 origin master:m2;   !ok;    /FATAL: C/
+                                            /DENIED by fallthru/
+    glt push u2 origin master:m1;   ok;     /To file:///foo/aa/
+                                            /8cd302a..29b8683/
+                                            /master -> m1/
+";