diff options
-rw-r--r-- | Bugzilla/Attachment.pm | 13 | ||||
-rw-r--r-- | Bugzilla/Bug.pm | 15 | ||||
-rw-r--r-- | Bugzilla/Chart.pm | 7 | ||||
-rw-r--r-- | Bugzilla/Constants.pm | 2 | ||||
-rw-r--r-- | Bugzilla/Flag.pm | 28 | ||||
-rw-r--r-- | Bugzilla/Template.pm | 4 | ||||
-rwxr-xr-x | attachment.cgi | 23 | ||||
-rwxr-xr-x | buglist.cgi | 2 | ||||
-rw-r--r-- | docs/en/xml/Bugzilla-Guide.xml | 4 | ||||
-rwxr-xr-x | editflagtypes.cgi | 21 | ||||
-rwxr-xr-x | editgroups.cgi | 4 | ||||
-rwxr-xr-x | post_bug.cgi | 9 | ||||
-rwxr-xr-x | relogin.cgi | 31 | ||||
-rw-r--r-- | t/002goodperl.t | 33 | ||||
-rw-r--r-- | template/en/default/filterexceptions.pl | 1 | ||||
-rw-r--r-- | template/en/default/global/messages.html.tmpl | 2 | ||||
-rw-r--r-- | template/en/default/pages/release-notes.html.tmpl | 12 | ||||
-rw-r--r-- | template/en/default/reports/report-table.csv.tmpl | 8 | ||||
-rw-r--r-- | template/en/default/request/email.txt.tmpl | 13 | ||||
-rwxr-xr-x | token.cgi | 2 |
20 files changed, 150 insertions, 84 deletions
diff --git a/Bugzilla/Attachment.pm b/Bugzilla/Attachment.pm index c4c1b28aa..1302fc716 100644 --- a/Bugzilla/Attachment.pm +++ b/Bugzilla/Attachment.pm @@ -972,10 +972,12 @@ sub get_content_type { return 'text/plain' if ($cgi->param('ispatch') || $cgi->param('attach_text')); my $content_type; - if (!defined $cgi->param('contenttypemethod')) { + my $method = $cgi->param('contenttypemethod'); + + if (!defined $method) { ThrowUserError("missing_content_type_method"); } - elsif ($cgi->param('contenttypemethod') eq 'autodetect') { + elsif ($method eq 'autodetect') { defined $cgi->upload('data') || ThrowUserError('file_not_specified'); # The user asked us to auto-detect the content type, so use the type # specified in the HTTP request headers. @@ -996,18 +998,17 @@ sub get_content_type { $content_type = 'image/png'; } } - elsif ($cgi->param('contenttypemethod') eq 'list') { + elsif ($method eq 'list') { # The user selected a content type from the list, so use their # selection. $content_type = $cgi->param('contenttypeselection'); } - elsif ($cgi->param('contenttypemethod') eq 'manual') { + elsif ($method eq 'manual') { # The user entered a content type manually, so use their entry. $content_type = $cgi->param('contenttypeentry'); } else { - ThrowCodeError("illegal_content_type_method", - { contenttypemethod => $cgi->param('contenttypemethod') }); + ThrowCodeError("illegal_content_type_method", { contenttypemethod => $method }); } return $content_type; } diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm index 962c3f36f..39a0f2596 100644 --- a/Bugzilla/Bug.pm +++ b/Bugzilla/Bug.pm @@ -1012,12 +1012,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 @@ -1040,6 +1034,9 @@ sub update { $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; @@ -1075,6 +1072,12 @@ sub update { $_->update foreach @{ $self->{_update_ref_bugs} || [] }; delete $self->{_update_ref_bugs}; + # Flags + my ($removed, $added) = Bugzilla::Flag->update_flags($self, $old_bug, $delta_ts); + if ($removed || $added) { + $changes->{'flagtypes.name'} = [$removed, $added]; + } + # BMO - allow extensions to alter what is logged into bugs_activity Bugzilla::Hook::process('bug_update_before_logging', { bug => $self, timestamp => $delta_ts, changes => $changes, old_bug => $old_bug }); diff --git a/Bugzilla/Chart.pm b/Bugzilla/Chart.pm index dfbf32a51..8fd4706e4 100644 --- a/Bugzilla/Chart.pm +++ b/Bugzilla/Chart.pm @@ -110,10 +110,9 @@ sub init { if ($self->{'datefrom'} && $self->{'dateto'} && $self->{'datefrom'} > $self->{'dateto'}) { - ThrowUserError("misarranged_dates", - {'datefrom' => $cgi->param('datefrom'), - 'dateto' => $cgi->param('dateto')}); - } + ThrowUserError('misarranged_dates', { 'datefrom' => scalar $cgi->param('datefrom'), + 'dateto' => scalar $cgi->param('dateto') }); + } } # Alter Chart so that the selected series are added to it. diff --git a/Bugzilla/Constants.pm b/Bugzilla/Constants.pm index 0c3c72a49..1383ca7fe 100644 --- a/Bugzilla/Constants.pm +++ b/Bugzilla/Constants.pm @@ -215,7 +215,7 @@ use Memoize; # CONSTANTS # # Bugzilla version -use constant BUGZILLA_VERSION => "4.2.9+"; +use constant BUGZILLA_VERSION => "4.2.11"; # A base link to the current REST Documentation. We place it here # as it will need to be updated to whatever the current release is. diff --git a/Bugzilla/Flag.pm b/Bugzilla/Flag.pm index d89a3adb4..e87f1f674 100644 --- a/Bugzilla/Flag.pm +++ b/Bugzilla/Flag.pm @@ -1045,18 +1045,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/Bugzilla/Template.pm b/Bugzilla/Template.pm index 6a46701ff..9bd0c51bd 100644 --- a/Bugzilla/Template.pm +++ b/Bugzilla/Template.pm @@ -850,10 +850,12 @@ sub create { }, # In CSV, quotes are doubled, and any value containing a quote or a - # comma is enclosed in quotes. + # comma is enclosed in quotes. If a field starts with an equals + # sign, it is proceed by a space. csv => sub { my ($var) = @_; + $var = ' ' . $var if substr($var, 0, 1) eq '='; $var =~ s/\"/\"\"/g; if ($var !~ /^-?(\d+\.)?\d*$/) { $var = "\"$var\""; diff --git a/attachment.cgi b/attachment.cgi index dff537e2b..3ffcda821 100755 --- a/attachment.cgi +++ b/attachment.cgi @@ -235,8 +235,9 @@ sub validateContext { my $context = $cgi->param('context') || "patch"; if ($context ne "file" && $context ne "patch") { - detaint_natural($context) - || ThrowUserError("invalid_context", { context => $cgi->param('context') }); + my $orig_context = $context; + detaint_natural($context) + || ThrowUserError("invalid_context", { context => $orig_context }); } return $context; @@ -545,25 +546,23 @@ sub insert { # Must be called before create() as it may alter $cgi->param('ispatch'). my $content_type = Bugzilla::Attachment::get_content_type(); - # Get the attach data - my $data = scalar($cgi->param('attach_text')); - if ($data) { + # Get the filehandle of the attachment. + my $data_fh = $cgi->upload('data'); + my $attach_text = $cgi->param('attach_text'); + + if ($attach_text) { # Convert to unix line-endings if pasting a patch if (scalar($cgi->param('ispatch'))) { - $data =~ s/[\012\015]{1,2}/\012/g; + $attach_text =~ s/[\012\015]{1,2}/\012/g; } } - else { - # Get the filehandle of the attachment. - $data = $cgi->upload('data'); - } my $attachment = Bugzilla::Attachment->create( {bug => $bug, creation_ts => $timestamp, - data => $data, + data => $attach_text || $data_fh, description => scalar $cgi->param('description'), - filename => $cgi->param('attach_text') ? "file_$bugid.txt" : scalar $cgi->upload('data'), + filename => $attach_text ? "file_$bugid.txt" : $data_fh, ispatch => scalar $cgi->param('ispatch'), isprivate => scalar $cgi->param('isprivate'), mimetype => $content_type, diff --git a/buglist.cgi b/buglist.cgi index d6875f5fe..f2cc0a53c 100755 --- a/buglist.cgi +++ b/buglist.cgi @@ -980,7 +980,7 @@ if (scalar(@products) == 1) { # This is used in the "Zarroo Boogs" case. elsif (my @product_input = $cgi->param('product')) { if (scalar(@product_input) == 1 and $product_input[0] ne '') { - $one_product = new Bugzilla::Product({ name => $cgi->param('product') }); + $one_product = new Bugzilla::Product({ name => $product_input[0] }); } } # We only want the template to use it if the user can actually diff --git a/docs/en/xml/Bugzilla-Guide.xml b/docs/en/xml/Bugzilla-Guide.xml index 77f48da2e..d77d14e27 100644 --- a/docs/en/xml/Bugzilla-Guide.xml +++ b/docs/en/xml/Bugzilla-Guide.xml @@ -32,9 +32,9 @@ For a devel release, simple bump bz-ver and bz-date --> -<!ENTITY bz-ver "4.2.9+"> +<!ENTITY bz-ver "4.2.11"> <!ENTITY bz-nextver "4.4"> -<!ENTITY bz-date "2014-04-18"> +<!ENTITY bz-date "2014-10-06"> <!ENTITY current-year "2014"> <!ENTITY landfillbase "http://landfill.bugzilla.org/bugzilla-4.2-branch/"> diff --git a/editflagtypes.cgi b/editflagtypes.cgi index d75bebba2..d28188ad7 100755 --- a/editflagtypes.cgi +++ b/editflagtypes.cgi @@ -59,23 +59,24 @@ my @products = @{$vars->{products}}; my $action = $cgi->param('action') || 'list'; my $token = $cgi->param('token'); -my $product = $cgi->param('product'); -my $component = $cgi->param('component'); +my $prod_name = $cgi->param('product'); +my $comp_name = $cgi->param('component'); my $flag_id = $cgi->param('id'); -if ($product) { +my ($product, $component); + +if ($prod_name) { # Make sure the user is allowed to view this product name. # Users with global editcomponents privs can see all product names. - ($product) = grep { lc($_->name) eq lc($product) } @products; - $product || ThrowUserError('product_access_denied', { name => $cgi->param('product') }); + ($product) = grep { lc($_->name) eq lc($prod_name) } @products; + $product || ThrowUserError('product_access_denied', { name => $prod_name }); } -if ($component) { - ($product && $product->id) - || ThrowUserError('flag_type_component_without_product'); - ($component) = grep { lc($_->name) eq lc($component) } @{$product->components}; +if ($comp_name) { + $product || ThrowUserError('flag_type_component_without_product'); + ($component) = grep { lc($_->name) eq lc($comp_name) } @{$product->components}; $component || ThrowUserError('product_unknown_component', { product => $product->name, - comp => $cgi->param('component') }); + comp => $comp_name }); } # If 'categoryAction' is set, it has priority over 'action'. diff --git a/editgroups.cgi b/editgroups.cgi index a879aa770..ccd0bd432 100755 --- a/editgroups.cgi +++ b/editgroups.cgi @@ -242,7 +242,7 @@ if ($action eq 'new') { if ($action eq 'del') { # Check that an existing group ID is given - my $group = Bugzilla::Group->check({ id => $cgi->param('group') }); + my $group = Bugzilla::Group->check({ id => scalar $cgi->param('group') }); $group->check_remove({ test_only => 1 }); $vars->{'shared_queries'} = $dbh->selectrow_array('SELECT COUNT(*) @@ -266,7 +266,7 @@ if ($action eq 'del') { if ($action eq 'delete') { check_token_data($token, 'delete_group'); # Check that an existing group ID is given - my $group = Bugzilla::Group->check({ id => $cgi->param('group') }); + my $group = Bugzilla::Group->check({ id => scalar $cgi->param('group') }); $vars->{'name'} = $group->name; $group->remove_from_db({ remove_from_users => scalar $cgi->param('removeusers'), diff --git a/post_bug.cgi b/post_bug.cgi index 21b621acc..006fd40ee 100755 --- a/post_bug.cgi +++ b/post_bug.cgi @@ -187,7 +187,10 @@ if (defined $cgi->param('version')) { # after the bug is filed. # Add an attachment if requested. -if (defined($cgi->upload('data')) || $cgi->param('attach_text')) { +my $data_fh = $cgi->upload('data'); +my $attach_text = $cgi->param('attach_text'); + +if ($data_fh || $attach_text) { $cgi->param('isprivate', $cgi->param('comment_is_private')); # Must be called before create() as it may alter $cgi->param('ispatch'). @@ -202,9 +205,9 @@ if (defined($cgi->upload('data')) || $cgi->param('attach_text')) { $attachment = Bugzilla::Attachment->create( {bug => $bug, creation_ts => $timestamp, - data => scalar $cgi->param('attach_text') || $cgi->upload('data'), + data => $attach_text || $data_fh, description => scalar $cgi->param('description'), - filename => $cgi->param('attach_text') ? "file_$id.txt" : scalar $cgi->upload('data'), + filename => $attach_text ? "file_$id.txt" : $data_fh, ispatch => scalar $cgi->param('ispatch'), isprivate => scalar $cgi->param('isprivate'), mimetype => $content_type, diff --git a/relogin.cgi b/relogin.cgi index 295d199c5..6eb798205 100755 --- a/relogin.cgi +++ b/relogin.cgi @@ -87,19 +87,21 @@ elsif ($action eq 'begin-sudo') { { $credentials_provided = 1; } - + # Next, log in the user my $user = Bugzilla->login(LOGIN_REQUIRED); - + + my $target_login = $cgi->param('target_login'); + my $reason = $cgi->param('reason') || ''; + # At this point, the user is logged in. However, if they used a method # where they could have provided a username/password (i.e. CGI), but they # did not provide a username/password, then throw an error. if ($user->authorizer->can_login && !$credentials_provided) { ThrowUserError('sudo_password_required', - { target_login => $cgi->param('target_login'), - reason => $cgi->param('reason')}); + { target_login => $target_login, reason => $reason }); } - + # The user must be in the 'bz_sudoers' group unless ($user->in_group('bz_sudoers')) { ThrowUserError('auth_failure', { group => 'bz_sudoers', @@ -123,30 +125,22 @@ elsif ($action eq 'begin-sudo') { && ($token_data eq 'sudo_prepared')) { ThrowUserError('sudo_preparation_required', - { target_login => scalar $cgi->param('target_login'), - reason => scalar $cgi->param('reason')}); + { target_login => $target_login, reason => $reason }); } delete_token($cgi->param('token')); # Get & verify the target user (the user who we will be impersonating) - my $target_user = - new Bugzilla::User({ name => $cgi->param('target_login') }); + my $target_user = new Bugzilla::User({ name => $target_login }); unless (defined($target_user) && $target_user->id && $user->can_see_user($target_user)) { - ThrowUserError('user_match_failed', - { 'name' => $cgi->param('target_login') } - ); + ThrowUserError('user_match_failed', { name => $target_login }); } if ($target_user->in_group('bz_sudo_protect')) { ThrowUserError('sudo_protected', { login => $target_user->login }); } - # If we have a reason passed in, keep it under 200 characters - my $reason = $cgi->param('reason') || ''; - $reason = substr($reason, 0, 200); - # Calculate the session expiry time (T + 6 hours) my $time_string = time2str('%a, %d-%b-%Y %T %Z', time + MAX_SUDO_TOKEN_AGE, 'GMT'); @@ -166,9 +160,12 @@ elsif ($action eq 'begin-sudo') { # For the present, change the values of Bugzilla::user & Bugzilla::sudoer Bugzilla->sudo_request($target_user, $user); - + # NOTE: If you want to log the start of an sudo session, do it here. + # If we have a reason passed in, keep it under 200 characters + $reason = substr($reason, 0, 200); + # Go ahead and send out the message now my $message; my $mail_template = Bugzilla->template_inner($target_user->setting('lang')); diff --git a/t/002goodperl.t b/t/002goodperl.t index e9726cb8c..77b014f6a 100644 --- a/t/002goodperl.t +++ b/t/002goodperl.t @@ -32,7 +32,7 @@ use lib 't'; use Support::Files; -use Test::More tests => (scalar(@Support::Files::testitems) * 3); +use Test::More tests => (scalar(@Support::Files::testitems) * 4); my @testitems = @Support::Files::testitems; # get the files to test. @@ -126,4 +126,35 @@ foreach my $file (@testitems) { close(FILE); } + +# Forbird the { foo => $cgi->param() } syntax, for security reasons. +foreach my $file (@testitems) { + $file =~ s/\s.*$//; # nuke everything after the first space (#comment) + next unless $file; # skip null entries + if (!open(FILE, $file)) { + ok(0, "could not open $file --WARNING"); + next; + } + my $lineno = 0; + my @unsafe_args; + + while (my $file_line = <FILE>) { + $lineno++; + $file_line =~ s/^\s*(.+)\s*$/$1/; # Remove leading and trailing whitespaces. + if ($file_line =~ /^[^#]+=> \$cgi\->param/) { + push(@unsafe_args, "$file_line on line $lineno"); + } + } + + if (@unsafe_args) { + ok(0, "$file incorrectly passes a CGI argument to a hash --ERROR\n" . + join("\n", @unsafe_args)); + } + else { + ok(1, "$file has no vulnerable hash syntax"); + } + + close(FILE); +} + exit 0; diff --git a/template/en/default/filterexceptions.pl b/template/en/default/filterexceptions.pl index 84ff45db7..a02230950 100644 --- a/template/en/default/filterexceptions.pl +++ b/template/en/default/filterexceptions.pl @@ -185,7 +185,6 @@ ], 'global/messages.html.tmpl' => [ - 'message_tag', 'series.frequency * 2', ], diff --git a/template/en/default/global/messages.html.tmpl b/template/en/default/global/messages.html.tmpl index 7feb54583..0f408842f 100644 --- a/template/en/default/global/messages.html.tmpl +++ b/template/en/default/global/messages.html.tmpl @@ -942,7 +942,7 @@ [% IF !message %] [% message = BLOCK %] You are using [% terms.Bugzilla %]'s messaging functions incorrectly. You - passed in the string '[% message_tag %]'. The correct use is to pass + passed in the string '[% message_tag FILTER html %]'. The correct use is to pass in a tag, and define that tag in the file messages.html.tmpl.<br> <br> If you are a [% terms.Bugzilla %] end-user seeing this message, please diff --git a/template/en/default/pages/release-notes.html.tmpl b/template/en/default/pages/release-notes.html.tmpl index f06c7450e..690d334ae 100644 --- a/template/en/default/pages/release-notes.html.tmpl +++ b/template/en/default/pages/release-notes.html.tmpl @@ -53,6 +53,18 @@ <h2 id="v42_point">Updates in this 4.2.x Release</h2> +<h3>4.2.11</h3> + +<p>This release fixes several security issues. See the + <a href="http://www.bugzilla.org/security/4.0.14/">Security Advisory</a> + for details.</p> + +<h3>4.2.10</h3> + +<p>This release fixes one security issue. See the + <a href="http://www.bugzilla.org/security/4.0.13/">Security Advisory</a> + for details.</p> + <h3>4.2.9</h3> <p>This release fixes one regression introduced in [% terms.Bugzilla %] 4.2.8 by diff --git a/template/en/default/reports/report-table.csv.tmpl b/template/en/default/reports/report-table.csv.tmpl index 4d8b50a85..c978cf981 100644 --- a/template/en/default/reports/report-table.csv.tmpl +++ b/template/en/default/reports/report-table.csv.tmpl @@ -39,11 +39,13 @@ [% END %] [% tbl_field_disp FILTER csv %]: [% tbl_disp FILTER csv %] [% END %] -[% IF row_field %] +[% IF row_field && col_field %] + [% row_field_disp _ ' / ' _ col_field_disp FILTER csv %] +[% ELSIF row_field %] [% row_field_disp FILTER csv %] +[% ELSE %] + [% col_field_disp FILTER csv %] [% END %] -[% " / " IF col_field AND row_field %] -[% col_field_disp FILTER csv %] [% IF col_field -%] [% FOREACH col = col_names -%] [% colsepchar %] diff --git a/template/en/default/request/email.txt.tmpl b/template/en/default/request/email.txt.tmpl index 1fed168cb..6241dd34f 100644 --- a/template/en/default/request/email.txt.tmpl +++ b/template/en/default/request/email.txt.tmpl @@ -85,11 +85,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 %] @@ -385,7 +385,7 @@ sub confirm_create_account { my $otheruser = Bugzilla::User->create({ login_name => $login_name, - realname => $cgi->param('realname'), + realname => scalar $cgi->param('realname'), cryptpassword => $password}); # Now delete this token. |