Bug 781850 (CVE-2012-4198): [SECURITY] Do not leak the existence of groups when using User.get()

r=dkl a=LpSolit

2 files changed, 26 insertions, 11 deletions

diff --git a/Bugzilla/WebService/Constants.pm b/Bugzilla/WebService/Constants.pm
index 2ffad430c..a5a5dffe9 100644
--- a/Bugzilla/WebService/Constants.pm
+++ b/Bugzilla/WebService/Constants.pm
@@ -158,6 +158,7 @@ use constant WS_ERROR_CODE => {
     group_exists => 801,
     empty_group_description => 802,
     invalid_regexp => 803,
+    invalid_group_name => 804,
 
     # Classification errors are 900-1000
     auth_classification_not_enabled => 900,
diff --git a/Bugzilla/WebService/User.pm b/Bugzilla/WebService/User.pm
index 8af10a224..527ca95a3 100644
--- a/Bugzilla/WebService/User.pm
+++ b/Bugzilla/WebService/User.pm
@@ -310,17 +310,23 @@ sub _filter_users_by_group {
     # If no groups are specified, we return all users.
     return $users if (!$group_ids and !$group_names);
 
-    my @groups = map { Bugzilla::Group->check({ id => $_ }) } 
-                     @{ $group_ids || [] };
-    my @name_groups = map { Bugzilla::Group->check($_) } 
-                          @{ $group_names || [] };
-    my %unique_groups;
-    foreach my $group (@groups, @name_groups) {
-        $unique_groups{$group->id} ||= $group;
+    my $user = Bugzilla->user;
+    my (@groups, %groups);
+
+    if ($group_ids) {
+        @groups = map { Bugzilla::Group->check({ id => $_ }) } @$group_ids;
+        $groups{$_->id} = $_ foreach @groups;
+    }
+    if ($group_names) {
+        foreach my $name (@$group_names) {
+            my $group = Bugzilla::Group->check({ name => $name, _error => 'invalid_group_name' });
+            $user->in_group($group) || ThrowUserError('invalid_group_name', { name => $name });
+            $groups{$group->id} = $group;
+        }
     }
+    @groups = values %groups;
 
-    my @in_group = grep { $self->_user_in_any_group($_, [values %unique_groups]) }
-                        @$users;
+    my @in_group = grep { $self->_user_in_any_group($_, \@groups) } @$users;
     return \@in_group;
 }
 
@@ -875,10 +881,10 @@ querying your own account, even if you are in the editusers group.
 
 =over
 
-=item 51 (Bad Login Name or Group Name)
+=item 51 (Bad Login Name or Group ID)
 
 You passed an invalid login name in the "names" array or a bad
-group name/id in the C<groups>/C<group_ids> arguments.
+group ID in the C<group_ids> argument.
 
 =item 304 (Authorization Required)
 
@@ -890,6 +896,11 @@ wanted to get information about by user id.
 Logged-out users cannot use the "ids" or "match" arguments to this 
 function.
 
+=item 804 (Invalid Group Name)
+
+You passed a group name in the C<groups> argument which either does not
+exist or you do not belong to it.
+
 =back
 
 =item B<History>
@@ -903,6 +914,9 @@ function.
 =item C<include_disabled> was added in Bugzilla B<4.0>. Default
 behavior for C<match> was changed to only return enabled accounts.
 
+=item Error 804 has been added in Bugzilla 4.0.9 and 4.2.4. It's now
+illegal to pass a group name you don't belong to.
+
 =item C<groups>, C<saved_searches>, and C<saved_reports> were added
 in Bugzilla B<4.4>.