Bug 141593 You can add/remove dependencies on bugs you can't see

Patch by Joel Peshkin <bugreport@peshkin.net>
r=lpsolit, a=justdave

3 files changed, 33 insertions, 8 deletions

diff --git a/post_bug.cgi b/post_bug.cgi
index 176b42d71..18faa5090 100755
--- a/post_bug.cgi
+++ b/post_bug.cgi
@@ -261,7 +261,9 @@ foreach my $field ("dependson", "blocked") {
         my @validvalues;
         foreach my $id (split(/[\s,]+/, $cgi->param($field))) {
             next unless $id;
-            ValidateBugID($id, $field);
+            # $field is not passed to ValidateBugID to prevent adding new 
+            # dependencies on inacessible bugs.
+            ValidateBugID($id);
             push(@validvalues, $id);
         }
         $cgi->param(-name => $field, -value => join(",", @validvalues));
diff --git a/process_bug.cgi b/process_bug.cgi
index 0cc4a224f..adb6a3ded 100755
--- a/process_bug.cgi
+++ b/process_bug.cgi
@@ -43,6 +43,7 @@ use strict;
 my $UserInEditGroupSet = -1;
 my $UserInCanConfirmGroupSet = -1;
 my $PrivilegesRequired = 0;
+my $lastbugid = 0;
 
 use lib qw(.);
 
@@ -144,14 +145,32 @@ ValidateComment(scalar $cgi->param('comment'));
 # is a bug alias that gets converted to its corresponding bug ID
 # during validation.
 foreach my $field ("dependson", "blocked") {
-    if ($cgi->param($field)) {
-        my @validvalues;
+    if ($cgi->param('id')) {
+        my $bug = new Bugzilla::Bug($cgi->param('id'), $user->id);
+        my @old = @{$bug->$field};
+        my @new;
         foreach my $id (split(/[\s,]+/, $cgi->param($field))) {
             next unless $id;
             ValidateBugID($id, $field);
-            push(@validvalues, $id);
+            push @new, $id;
+        }
+        $cgi->param($field, join(",", @new));
+        my ($added, $removed) = Bugzilla::Util::diff_arrays(\@old, \@new);
+        foreach my $id (@$added , @$removed) {
+            # ValidateBugID is called without $field here so that it will
+            # throw an error if any of the changed bugs are not visible.
+            ValidateBugID($id);
+            if (!CheckCanChangeField($field, $bug->bug_id, 0, 1)) {
+                $vars->{'privs'} = $PrivilegesRequired;
+                $vars->{'field'} = $field;
+                ThrowUserError("illegal_change", $vars);
+            }
         }
-        $cgi->param($field, join(",", @validvalues));
+    } else {
+        # Bugzilla does not support mass-change of dependencies so they
+        # are not validated.  To prevent a URL-hacking risk, the dependencies
+        # are deleted for mass-changes.
+        $cgi->delete($field);
     }
 }
 
@@ -353,7 +372,6 @@ if (((defined $cgi->param('id') && $cgi->param('product') ne $oldproduct)
 # now, the rules are pretty simple, and don't look at the field itself very
 # much, but that could be enhanced.
 
-my $lastbugid = 0;
 my $ownerid;
 my $reporterid;
 my $qacontactid;
diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl
index 8646100da..09f3d6e8a 100644
--- a/template/en/default/global/user-error.html.tmpl
+++ b/template/en/default/global/user-error.html.tmpl
@@ -524,8 +524,13 @@
     [% title = "Not allowed" %]
     You tried to change the 
     <strong>[% field_descs.$field FILTER html %]</strong> field 
-    from <em>[% oldvalue FILTER html %]</em> to 
-    <em>[% newvalue FILTER html %]</em>, but only
+    [% IF oldvalue %]
+      from <em>[% oldvalue FILTER html %]</em>
+    [% END %]
+    [% IF newvalue %]
+      to <em>[% newvalue FILTER html %]</em>
+    [% END %]
+    , but only
     [% IF privs < 3 %]
       the assignee
       [% IF privs < 2 %] or reporter [% END %]