compile: (oopsies...) plug security hole in delegation feature
I was trying to determine how close gitolite can come to the ACL model of a proprietary product called codebeamer, and one of the items was how to make a "role" (like QA_Lead) have different "members" in different projects. I then realised delegation already does that! Which is great, but as I thought about it more, I realised... well, we'll let the in-code comments speak for themselves :-) Anyway, all it needed was a 1-line fix, luckily... <phew> And it would have only affected people who use delegation.
This commit is contained in:
parent
cba66c6e5a
commit
516c028b81
|
@ -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
|
# $groups{group}{member} = "master" (or name of fragment file in which the
|
||||||
# group is defined).
|
# group is defined).
|
||||||
my %groups = ();
|
our %groups = ();
|
||||||
|
|
||||||
# %repos has two functions.
|
# %repos has two functions.
|
||||||
|
|
||||||
|
@ -282,6 +282,24 @@ parse_conf_file($GL_CONF, 'master');
|
||||||
wrap_chdir($GL_ADMINDIR);
|
wrap_chdir($GL_ADMINDIR);
|
||||||
for my $fragment_file (glob("conf/fragments/*.conf"))
|
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;
|
my $fragment = $fragment_file;
|
||||||
$fragment =~ s/^conf\/fragments\/(.*).conf$/$1/;
|
$fragment =~ s/^conf\/fragments\/(.*).conf$/$1/;
|
||||||
parse_conf_file($fragment_file, $fragment);
|
parse_conf_file($fragment_file, $fragment);
|
||||||
|
|
Loading…
Reference in a new issue