summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorFrédéric Buclin <LpSolit@gmail.com>2014-10-06 16:29:01 +0200
committerDavid Lawrence <dkl@mozilla.com>2014-10-06 16:29:01 +0200
commit9e186bdd5da79077f162351d61fd1163d6cfd622 (patch)
tree3ddcb53698d5f608dd9228b1632481f4a0fcc04f
parent553568ddf8d9c6282daf779bb83dec7111ed4ff0 (diff)
downloadbugzilla-9e186bdd5da79077f162351d61fd1163d6cfd622.tar.gz
bugzilla-9e186bdd5da79077f162351d61fd1163d6cfd622.tar.xz
Bug 1075578: [SECURITY] Improper filtering of CGI arguments
r=dkl,a=sgreen
-rw-r--r--Bugzilla/Chart.pm7
-rwxr-xr-xattachment.cgi10
-rwxr-xr-xbuglist.cgi2
-rwxr-xr-xeditflagtypes.cgi21
-rwxr-xr-xeditgroups.cgi4
-rwxr-xr-xpost_bug.cgi9
-rwxr-xr-xrelogin.cgi31
-rw-r--r--template/en/default/filterexceptions.pl1
-rw-r--r--template/en/default/global/messages.html.tmpl2
-rwxr-xr-xtoken.cgi2
-rwxr-xr-xuserprefs.cgi2
11 files changed, 46 insertions, 45 deletions
diff --git a/Bugzilla/Chart.pm b/Bugzilla/Chart.pm
index c8cd41b52..3c69006aa 100644
--- a/Bugzilla/Chart.pm
+++ b/Bugzilla/Chart.pm
@@ -96,10 +96,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/attachment.cgi b/attachment.cgi
index f2eca5694..3d0ac2bb6 100755
--- a/attachment.cgi
+++ b/attachment.cgi
@@ -196,8 +196,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;
@@ -515,13 +516,14 @@ sub insert {
# Get the filehandle of the attachment.
my $data_fh = $cgi->upload('data');
+ my $attach_text = $cgi->param('attach_text');
my $attachment = Bugzilla::Attachment->create(
{bug => $bug,
creation_ts => $timestamp,
- data => scalar $cgi->param('attach_text') || $data_fh,
+ 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 5e84b340b..daee34c9b 100755
--- a/buglist.cgi
+++ b/buglist.cgi
@@ -945,7 +945,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 = Bugzilla::Product->new({ name => $cgi->param('product'), cache => 1 });
+ $one_product = Bugzilla::Product->new({ name => $product_input[0], cache => 1 });
}
}
# We only want the template to use it if the user can actually
diff --git a/editflagtypes.cgi b/editflagtypes.cgi
index 06144fc5c..d848d250a 100755
--- a/editflagtypes.cgi
+++ b/editflagtypes.cgi
@@ -41,23 +41,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 9c33a0ee3..287ac1114 100755
--- a/editgroups.cgi
+++ b/editgroups.cgi
@@ -224,7 +224,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(*)
@@ -248,7 +248,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 f73ca6b29..9da8faec1 100755
--- a/post_bug.cgi
+++ b/post_bug.cgi
@@ -152,7 +152,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').
@@ -167,9 +170,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 fd1f58ea0..6f0c970f1 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/template/en/default/filterexceptions.pl b/template/en/default/filterexceptions.pl
index 93c4a42a9..3c5bfc217 100644
--- a/template/en/default/filterexceptions.pl
+++ b/template/en/default/filterexceptions.pl
@@ -167,7 +167,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 f47a1d6ec..3a8aa1ada 100644
--- a/template/en/default/global/messages.html.tmpl
+++ b/template/en/default/global/messages.html.tmpl
@@ -943,7 +943,7 @@
[% IF !message %]
[% message = BLOCK %]
You are using 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 <kbd>messages.html.tmpl</kbd>.<br>
<br>
If you are a [% terms.Bugzilla %] end-user seeing this message, please
diff --git a/token.cgi b/token.cgi
index 145aa5225..830ecfccb 100755
--- a/token.cgi
+++ b/token.cgi
@@ -313,7 +313,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.
diff --git a/userprefs.cgi b/userprefs.cgi
index ad5fb7d19..1f5f625f7 100755
--- a/userprefs.cgi
+++ b/userprefs.cgi
@@ -544,7 +544,7 @@ sub SaveApiKey {
if ($cgi->param('new_key')) {
$vars->{new_key} = Bugzilla::User::APIKey->create({
user_id => $user->id,
- description => $cgi->param('new_description'),
+ description => scalar $cgi->param('new_description'),
});
# As a security precaution, we always sent out an e-mail when