From 4eb4a65a1a945e558086603e75b367bc7bd1d971 Mon Sep 17 00:00:00 2001 From: Frédéric Buclin Date: Thu, 26 Jul 2012 23:01:12 +0200 Subject: Bug 777398: (CVE-2012-1968) [SECURITY] HTML bugmail exposes information about restricted bugs r=glob a=LpSolit --- Bugzilla/Hook.pm | 6 ++++++ Bugzilla/Template.pm | 32 ++++++++++++++++------------- template/en/default/email/bugmail.html.tmpl | 21 ++++++++++--------- 3 files changed, 35 insertions(+), 24 deletions(-) diff --git a/Bugzilla/Hook.pm b/Bugzilla/Hook.pm index 27184c2e4..730170663 100644 --- a/Bugzilla/Hook.pm +++ b/Bugzilla/Hook.pm @@ -411,6 +411,12 @@ Sometimes this is C, meaning that we are parsing text that is not a bug comment (but could still be some other part of a bug, like the summary line). +=item C + +The L object representing the user who will see the text. +This is useful to determine how much confidential information can be displayed +to the user. + =back =head2 bug_url_sub_classes diff --git a/Bugzilla/Template.pm b/Bugzilla/Template.pm index a2efec48c..87c8696b7 100644 --- a/Bugzilla/Template.pm +++ b/Bugzilla/Template.pm @@ -139,8 +139,9 @@ sub get_format { # If you want to modify this routine, read the comments carefully sub quoteUrls { - my ($text, $bug, $comment) = (@_); + my ($text, $bug, $comment, $user) = @_; return $text unless $text; + $user ||= Bugzilla->user; # We use /g for speed, but uris can have other things inside them # (http://foo/bug#3 for example). Filtering that out filters valid @@ -170,7 +171,7 @@ sub quoteUrls { my @hook_regexes; Bugzilla::Hook::process('bug_format_comment', { text => \$text, bug => $bug, regexes => \@hook_regexes, - comment => $comment }); + comment => $comment, user => $user }); foreach my $re (@hook_regexes) { my ($match, $replace) = @$re{qw(match replace)}; @@ -192,7 +193,7 @@ sub quoteUrls { map { qr/$_/ } grep($_, Bugzilla->params->{'urlbase'}, Bugzilla->params->{'sslbase'})) . ')'; $text =~ s~\b(${urlbase_re}\Qshow_bug.cgi?id=\E([0-9]+)(\#c([0-9]+))?)\b - ~($things[$count++] = get_bug_link($3, $1, { comment_num => $5 })) && + ~($things[$count++] = get_bug_link($3, $1, { comment_num => $5, user => $user })) && ("\0\0" . ($count-1) . "\0\0") ~egox; @@ -221,7 +222,7 @@ sub quoteUrls { # attachment links $text =~ s~\b(attachment\s*\#?\s*(\d+)(?:\s+\[details\])?) - ~($things[$count++] = get_attachment_link($2, $1)) && + ~($things[$count++] = get_attachment_link($2, $1, $user)) && ("\0\0" . ($count-1) . "\0\0") ~egmxi; @@ -238,7 +239,7 @@ sub quoteUrls { $text =~ s~\b($bug_re(?:\s*,?\s*$comment_re)?|$comment_re) ~ # We have several choices. $1 here is the link, and $2-4 are set # depending on which part matched - (defined($2) ? get_bug_link($2, $1, { comment_num => $3 }) : + (defined($2) ? get_bug_link($2, $1, { comment_num => $3, user => $user }) : "$1") ~egox; @@ -247,7 +248,7 @@ sub quoteUrls { $text =~ s~(?<=^\*\*\*\ This\ bug\ has\ been\ marked\ as\ a\ duplicate\ of\ ) (\d+) (?=\ \*\*\*\Z) - ~get_bug_link($1, $1) + ~get_bug_link($1, $1, { user => $user }) ~egmx; # Now remove the encoding hacks in reverse order @@ -261,15 +262,18 @@ sub quoteUrls { # Creates a link to an attachment, including its title. sub get_attachment_link { - my ($attachid, $link_text) = @_; + my ($attachid, $link_text, $user) = @_; my $dbh = Bugzilla->dbh; + $user ||= Bugzilla->user; my $attachment = new Bugzilla::Attachment($attachid); if ($attachment) { my $title = ""; my $className = ""; - if (Bugzilla->user->can_see_bug($attachment->bug_id)) { + if ($user->can_see_bug($attachment->bug_id) + && (!$attachment->isprivate || $user->is_insider)) + { $title = $attachment->description; } if ($attachment->isobsolete) { @@ -309,6 +313,7 @@ sub get_attachment_link { sub get_bug_link { my ($bug, $link_text, $options) = @_; $options ||= {}; + $options->{user} ||= Bugzilla->user; my $dbh = Bugzilla->dbh; if (defined $bug) { @@ -688,10 +693,10 @@ sub create { clean_text => \&Bugzilla::Util::clean_text , quoteUrls => [ sub { - my ($context, $bug, $comment) = @_; + my ($context, $bug, $comment, $user) = @_; return sub { my $text = shift; - return quoteUrls($text, $bug, $comment); + return quoteUrls($text, $bug, $comment, $user); }; }, 1 @@ -707,10 +712,9 @@ sub create { 1 ], - bug_list_link => sub - { - my $buglist = shift; - return join(", ", map(get_bug_link($_, $_), split(/ *, */, $buglist))); + bug_list_link => sub { + my ($buglist, $options) = @_; + return join(", ", map(get_bug_link($_, $_, $options), split(/ *, */, $buglist))); }, # In CSV, quotes are doubled, and any value containing a quote or a diff --git a/template/en/default/email/bugmail.html.tmpl b/template/en/default/email/bugmail.html.tmpl index cfb5a64e4..5d31d27a0 100644 --- a/template/en/default/email/bugmail.html.tmpl +++ b/template/en/default/email/bugmail.html.tmpl @@ -20,12 +20,12 @@ [% FOREACH comment = new_comments.reverse %]
[% IF comment.count %] - [% "Comment # ${comment.count}" FILTER bug_link( bug, - {comment_num => comment.count, full_url => 1}) FILTER none %] - on [% "$terms.bug $bug.id" FILTER bug_link( bug, { full_url => 1 }) FILTER none %] + [% "Comment # ${comment.count}" FILTER bug_link(bug, + {comment_num => comment.count, full_url => 1, user => to_user}) FILTER none %] + on [% "$terms.bug $bug.id" FILTER bug_link(bug, { full_url => 1, user => to_user }) FILTER none %] from [% INCLUDE global/user.html.tmpl who = comment.author %] [% END %] - 
[% comment.body_full({ wrap => 1 }) FILTER quoteUrls(bug, comment) %]
+ 
[% comment.body_full({ wrap => 1 }) FILTER quoteUrls(bug, comment, to_user) %]
[% END %]

@@ -58,13 +58,14 @@ [% SET in_table = 0 %] [% END %] [% IF change.blocker %] - [% "${terms.Bug} ${bug.id}" FILTER bug_link(bug, full_url => 1) FILTER none %] depends - on [% "${terms.bug} ${change.blocker.id}" - FILTER bug_link(change.blocker, full_url => 1) FILTER none %], + [% "${terms.Bug} ${bug.id}" FILTER bug_link(bug, {full_url => 1, user => to_user}) FILTER none %] + depends on + [%+ "${terms.bug} ${change.blocker.id}" + FILTER bug_link(change.blocker, {full_url => 1, user => to_user}) FILTER none %], which changed state. [% ELSE %] - [% INCLUDE global/user.html.tmpl who = change.who %] - changed [% "${terms.Bug} ${bug.id}" FILTER bug_link(bug, full_url => 1) FILTER none %] + [% INCLUDE global/user.html.tmpl who = change.who %] changed + [%+ "${terms.bug} ${bug.id}" FILTER bug_link(bug, {full_url => 1, user => to_user}) FILTER none %] [% END %]
[% IF in_table == 0 %] @@ -88,7 +89,7 @@ [% field_label FILTER html %] [% IF change.field_name == "bug_id" %] - [% new_value FILTER bug_link(bug, full_url => 1) FILTER none %] + [% new_value FILTER bug_link(bug, {full_url => 1, user => to_user}) FILTER none %] [% ELSE %] [% new_value FILTER html %] [% END %] -- cgit v1.2.3-24-g4f1b