diff --git a/src/gl-compile-conf b/src/gl-compile-conf index 247e5cc..77d340b 100755 --- a/src/gl-compile-conf +++ b/src/gl-compile-conf @@ -86,7 +86,7 @@ my $USERNAME_PATT=qr(^\@?[0-9a-zA-Z][0-9a-zA-Z._-]*$); # very simple patter # $groups{group}{member} = "master" (or name of fragment file in which the # group is defined). -my %groups = (); +our %groups = (); # %repos has two functions. @@ -282,6 +282,24 @@ parse_conf_file($GL_CONF, 'master'); wrap_chdir($GL_ADMINDIR); for my $fragment_file (glob("conf/fragments/*.conf")) { + # we already check (elsewhere) that a fragment called "foo" will not try + # to specify access control for a repo whose name is not "foo" or is not + # part of a group called "foo" created by master + + # meanwhile, I found a possible attack where the admin for group B creates + # a "convenience" group of (a subset of) his users, and then the admin for + # repo group A (alphabetically before B) adds himself to that same group + # in his own fragment. + + # as a result, admin_A now has access to group B repos :( + + # so now we lock the groups hash to the value it had after parsing + # "master", and localise any changes to it by this fragment so that they + # don't propagate to the next fragment. Thus, each fragment now has only + # those groups that are defined in "master" and itself + + local %groups = %groups; + my $fragment = $fragment_file; $fragment =~ s/^conf\/fragments\/(.*).conf$/$1/; parse_conf_file($fragment_file, $fragment);