Fix for bug 305773: fixes regression in flag validation code that let through untrusted requestees when multiple requestees were entered into a requestee field (per the new feature that lets multiple requestees be entered into those fields); r=lpsolit

1 files changed, 41 insertions, 35 deletions

diff --git a/Bugzilla/FlagType.pm b/Bugzilla/FlagType.pm
index 678721b5f..a7a32c5cc 100644
--- a/Bugzilla/FlagType.pm
+++ b/Bugzilla/FlagType.pm
@@ -352,6 +352,7 @@ sub validate {
 
     foreach my $id (@ids) {
         my $status = $cgi->param("flag_type-$id");
+        my @requestees = $cgi->param("requestee_type-$id");
         
         # Don't bother validating types the user didn't touch.
         next if $status eq "X";
@@ -374,48 +375,53 @@ sub validate {
         
         # Make sure the user didn't specify a requestee unless the flag
         # is specifically requestable.
-        my $new_requestee = trim($cgi->param("requestee_type-$id") || '');
-
         if ($status eq '?'
             && !$flag_type->{is_requesteeble}
-            && $new_requestee)
+            && scalar(@requestees) > 0)
         {
-            ThrowCodeError("flag_requestee_disabled",
-                           { name => $flag_type->{name} });
+            ThrowCodeError("flag_requestee_disabled", { type => $flag_type });
         }
 
-        # Make sure the requestee is authorized to access the bug
-        # (and attachment, if this installation is using the "insider group"
-        # feature and the attachment is marked private).
+        # Make sure the user didn't enter multiple requestees for a flag
+        # that can't be requested from more than one person at a time.
         if ($status eq '?'
-            && $flag_type->{is_requesteeble}
-            && $new_requestee)
+            && !$flag_type->{is_multiplicable}
+            && scalar(@requestees) > 1)
         {
-            # We know the requestee exists because we ran
-            # Bugzilla::User::match_field before getting here.
-            my $requestee = Bugzilla::User->new_from_login($new_requestee);
-
-            # Throw an error if the user can't see the bug.
-            if (!$requestee->can_see_bug($bug_id)) {
-                ThrowUserError("flag_requestee_unauthorized",
-                               { flag_type => $flag_type,
-                                 requestee => $requestee,
-                                 bug_id    => $bug_id,
-                                 attach_id => $attach_id });
-            }
-            
-            # Throw an error if the target is a private attachment and
-            # the requestee isn't in the group of insiders who can see it.
-            if ($attach_id
-                && Param("insidergroup")
-                && $cgi->param('isprivate')
-                && !$requestee->in_group(Param("insidergroup")))
-            {
-                ThrowUserError("flag_requestee_unauthorized_attachment",
-                               { flag_type => $flag_type,
-                                 requestee => $requestee,
-                                 bug_id    => $bug_id,
-                                 attach_id => $attach_id });
+            ThrowUserError("flag_not_multiplicable", { type => $flag_type });
+        }
+
+        # Make sure the requestees are authorized to access the bug
+        # (and attachment, if this installation is using the "insider group"
+        # feature and the attachment is marked private).
+        if ($status eq '?' && $flag_type->{is_requesteeble}) {
+            foreach my $login (@requestees) {
+                # We know the requestee exists because we ran
+                # Bugzilla::User::match_field before getting here.
+                my $requestee = Bugzilla::User->new_from_login($login);
+    
+                # Throw an error if the user can't see the bug.
+                if (!$requestee->can_see_bug($bug_id)) {
+                    ThrowUserError("flag_requestee_unauthorized",
+                                   { flag_type => $flag_type,
+                                     requestee => $requestee,
+                                     bug_id    => $bug_id,
+                                     attach_id => $attach_id });
+                }
+                
+                # Throw an error if the target is a private attachment and
+                # the requestee isn't in the group of insiders who can see it.
+                if ($attach_id
+                    && Param("insidergroup")
+                    && $cgi->param('isprivate')
+                    && !$requestee->in_group(Param("insidergroup")))
+                {
+                    ThrowUserError("flag_requestee_unauthorized_attachment",
+                                   { flag_type => $flag_type,
+                                     requestee => $requestee,
+                                     bug_id    => $bug_id,
+                                     attach_id => $attach_id });
+                }
             }
         }