aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSimon Green <sgreen@redhat.com>2014-10-06 14:47:38 +0000
committerDavid Lawrence <dkl@mozilla.com>2014-10-06 14:47:38 +0000
commita92eee1c158dc834613e54dc51f2ae3c85a4205b (patch)
tree3421a265605fb9a3874e27a6265c17a5940fbbe5
parentBug 1074980: Forbid the { foo => $cgi->param() } syntax to prevent data override (diff)
downloadbugzilla-a92eee1c158dc834613e54dc51f2ae3c85a4205b.tar.gz
bugzilla-a92eee1c158dc834613e54dc51f2ae3c85a4205b.tar.bz2
bugzilla-a92eee1c158dc834613e54dc51f2ae3c85a4205b.zip
Bug 1064140: [SECURITY] Private comments can be shown to flagmail recipients who aren't in the insider group
r=glob,a=glob
-rw-r--r--Bugzilla/Bug.pm15
-rw-r--r--Bugzilla/Flag.pm28
-rw-r--r--template/en/default/email/flagmail.txt.tmpl13
3 files changed, 38 insertions, 18 deletions
diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index d4d94b23f..d0e8f462b 100644
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -911,12 +911,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} || []}) {
# Override the Comment's timestamp to be identical to the update
@@ -939,6 +933,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;
@@ -971,6 +968,12 @@ sub update {
join(', ', map { $_->name } @$added_see)];
}
+ # Flags
+ my ($removed, $added) = Bugzilla::Flag->update_flags($self, $old_bug, $delta_ts);
+ if ($removed || $added) {
+ $changes->{'flagtypes.name'} = [$removed, $added];
+ }
+
$_->update foreach @{ $self->{_update_ref_bugs} || [] };
delete $self->{_update_ref_bugs};
diff --git a/Bugzilla/Flag.pm b/Bugzilla/Flag.pm
index ac06a255c..affeaee68 100644
--- a/Bugzilla/Flag.pm
+++ b/Bugzilla/Flag.pm
@@ -1007,18 +1007,32 @@ sub notify {
$default_lang = Bugzilla::User->new()->setting('lang');
}
+ # 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}->setting('lang') : $default_lang;
diff --git a/template/en/default/email/flagmail.txt.tmpl b/template/en/default/email/flagmail.txt.tmpl
index 169dfa892..037673dfc 100644
--- a/template/en/default/email/flagmail.txt.tmpl
+++ b/template/en/default/email/flagmail.txt.tmpl
@@ -64,11 +64,14 @@ Attachment [% attidsummary %]
[%- 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 %]