Bug 788098: Queries involving group substitution crash when usevisibilitygroups is enabled

r=dkl a=LpSolit

2 files changed, 14 insertions, 6 deletions

diff --git a/Bugzilla/Group.pm b/Bugzilla/Group.pm
index b7532fe09..382407748 100644
--- a/Bugzilla/Group.pm
+++ b/Bugzilla/Group.pm
@@ -189,7 +189,9 @@ sub check_members_are_visible {
     my $self = shift;
     my $user = Bugzilla->user;
     return if !Bugzilla->params->{'usevisibilitygroups'};
-    my $is_visible = grep { $_->id == $_ } @{ $user->visible_groups_inherited };
+
+    my $group_id = $self->id;
+    my $is_visible = grep { $_ == $group_id } @{ $user->visible_groups_inherited };
     if (!$is_visible) {
         ThrowUserError('group_not_visible', { group => $self });
     }
diff --git a/Bugzilla/Search.pm b/Bugzilla/Search.pm
index 9a5e888bc..f0e015cbc 100644
--- a/Bugzilla/Search.pm
+++ b/Bugzilla/Search.pm
@@ -2050,8 +2050,8 @@ sub _contact_pronoun {
     my ($self, $args) = @_;
     my $value = $args->{value};
     my $user = $self->_user;
-    
-    if ($value =~ /^\%group/) {
+
+    if ($value =~ /^\%group\.[^%]+%$/) {
         $self->_contact_exact_group($args);
     }
     elsif ($value =~ /^(%\w+%)$/) {
@@ -2068,11 +2068,17 @@ sub _contact_exact_group {
     my $dbh = Bugzilla->dbh;
     my $user = $self->_user;
     
+    # We already know $value will match this regexp, else we wouldn't be here.
     $value =~ /\%group\.([^%]+)%/;
-    my $group = Bugzilla::Group->check({ name => $1, _error => 'invalid_group_name' });
-    $group->check_members_are_visible();
+    my $group_name = $1;
+    my $group = Bugzilla::Group->check({ name => $group_name, _error => 'invalid_group_name' });
+    # Pass $group_name instead of $group->name to the error message
+    # to not leak the existence of the group.
     $user->in_group($group)
-      || ThrowUserError('invalid_group_name', {name => $group->name});
+      || ThrowUserError('invalid_group_name', { name => $group_name });
+    # Now that we know the user belongs to this group, it's safe
+    # to disclose more information.
+    $group->check_members_are_visible();
 
     my $group_ids = Bugzilla::Group->flatten_group_membership($group->id);
     my $table = "user_group_map_$chart_id";