From: Simon Green <sgreen@redhat.com>
Date: Mon, 6 Oct 2014 14:40:46 +0000 (+0000)
Subject: Bug 1064140: [SECURITY] Private comments can be shown to flagmail recipients who... 
X-Git-Tag: bugzilla-4.0.15~2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=19576a8290c0bb414cde2164b46e19d7f6532806;p=thirdparty%2Fbugzilla.git

Bug 1064140: [SECURITY] Private comments can be shown to flagmail recipients who aren't in the insider group
r=glob,a=glob
---

diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index 98d2825385..b57af4d39b 100644
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -888,12 +888,6 @@ sub update {
                                    join(', ', @added_names)];
     }
 
-    # Flags
-    my ($removed, $added) = Bugzilla::Flag->update_flags($self, $old_bug, $delta_ts);
-    if ($removed || $added) {
-        $changes->{'flagtypes.name'} = [$removed, $added];
-    }
-
     # Comments
     foreach my $comment (@{$self->{added_comments} || []}) {
         my $columns = join(',', keys %$comment);
@@ -918,6 +912,9 @@ sub update {
                          Bugzilla->user->id, $delta_ts, $comment_id);
     }
 
+    # Clear the cache of comments
+    delete $self->{comments};
+
     # Insert the values into the multiselect value tables
     my @multi_selects = grep {$_->type == FIELD_TYPE_MULTI_SELECT}
                              Bugzilla->active_custom_fields;
@@ -955,6 +952,12 @@ sub update {
                                 join(', ', @$added_see)];
     }
 
+    # Flags
+    my ($removed, $added) = Bugzilla::Flag->update_flags($self, $old_bug, $delta_ts);
+    if ($removed || $added) {
+        $changes->{'flagtypes.name'} = [$removed, $added];
+    }
+
     # Log bugs_activity items
     # XXX Eventually, when bugs_activity is able to track the dupe_id,
     # this code should go below the duplicates-table-updating code below.
diff --git a/Bugzilla/Flag.pm b/Bugzilla/Flag.pm
index d6dbe1629e..29483e908c 100644
--- a/Bugzilla/Flag.pm
+++ b/Bugzilla/Flag.pm
@@ -973,18 +973,32 @@ sub notify {
         $default_lang = Bugzilla::User->new()->settings->{'lang'}->{'value'};
     }
 
+    # Get comments on the bug
+    my $all_comments = $bug->comments({ after => $bug->lastdiffed });
+    @$all_comments   = grep { $_->type || $_->body =~ /\S/ } @$all_comments;
+
+    # Get public only comments
+    my $public_comments = [ grep { !$_->is_private } @$all_comments ];
+
     foreach my $to (keys %recipients) {
         # Add threadingmarker to allow flag notification emails to be the
         # threaded similar to normal bug change emails.
         my $thread_user_id = $recipients{$to} ? $recipients{$to}->id : 0;
 
-        my $vars = { 'flag'            => $flag,
-                     'old_flag'        => $old_flag,
-                     'to'              => $to,
-                     'date'            => $timestamp,
-                     'bug'             => $bug,
-                     'attachment'      => $attachment,
-                     'threadingmarker' => build_thread_marker($bug->id, $thread_user_id) };
+        # We only want to show private comments to users in the is_insider group
+        my $comments = $recipients{$to} && $recipients{$to}->is_insider
+            ? $all_comments : $public_comments;
+
+        my $vars = {
+            flag            => $flag,
+            old_flag        => $old_flag,
+            to              => $to,
+            date            => $timestamp,
+            bug             => $bug,
+            attachment      => $attachment,
+            threadingmarker => build_thread_marker($bug->id, $thread_user_id),
+            new_comments    => $comments,
+        };
 
         my $lang = $recipients{$to} ?
           $recipients{$to}->settings->{'lang'}->{'value'} : $default_lang;
diff --git a/template/en/default/request/email.txt.tmpl b/template/en/default/request/email.txt.tmpl
index 7cf75b867c..0d1feddaf5 100644
--- a/template/en/default/request/email.txt.tmpl
+++ b/template/en/default/request/email.txt.tmpl
@@ -77,11 +77,14 @@ Attachment [% attidsummary %]
 [%- END %]
 [%- FILTER bullet = wrap(80) %]
 
-[% USE Bugzilla %]
-[%-# .defined is necessary to avoid a taint issue in Perl < 5.10.1, see bug 509794. %]
-[% IF Bugzilla.cgi.param("comment").defined && Bugzilla.cgi.param("comment").length > 0 %]
-------- Additional Comments from [% user.identity %]
-[%+ Bugzilla.cgi.param("comment") FILTER strip_control_chars %]
+[% FOREACH comment = new_comments %]
+
+[%- IF comment.count %]
+--- Comment #[% comment.count %] from [% comment.author.identity %] ---
+[% ELSE %]
+--- Description ---
+[% END %]
+[%+ comment.body_full({ is_bugmail => 1, wrap => 1 }) FILTER strip_control_chars %]
 [% END %]
 
 [%- END %]