Skip to content

Commit

Permalink
Bug 1532406 - Removed useless trick_taint() and untaint() calls
Browse files Browse the repository at this point in the history
  • Loading branch information
dylanwh authored Mar 4, 2019
1 parent d929f71 commit 470728e
Show file tree
Hide file tree
Showing 84 changed files with 33 additions and 278 deletions.
3 changes: 0 additions & 3 deletions Bugzilla.pm
Original file line number Diff line number Diff line change
Expand Up @@ -602,9 +602,6 @@ sub log_user_request {
$user_id, remote_ip(), $user_agent, $request_url, $method,
$bug_id, $attach_id, $action, $server
);
foreach my $param (@params) {
trick_taint($param) if defined $param;
}

eval {
local request_cache->{dbh};
Expand Down
2 changes: 0 additions & 2 deletions Bugzilla/App/CGI.pm
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ use Mojo::Base 'Mojolicious::Controller';

use CGI::Compile;
use Try::Tiny;
use Taint::Util qw(untaint);
use Sys::Hostname;
use Sub::Quote 2.005000;
use Sub::Name;
Expand Down Expand Up @@ -52,7 +51,6 @@ sub load_one {
my $package = __PACKAGE__ . "::$name", my $inner_name = "_$name";
my $content = path(bz_locations->{cgi_path}, $file)->slurp;
$content = "package $package; $content";
untaint($content);
my %options = (package => $package, file => $file, line => 1, no_defer => 1,);
die "Tried to load $file more than once" if $SEEN{$file}++;
my $inner = quote_sub $inner_name, $content, {}, \%options;
Expand Down
2 changes: 0 additions & 2 deletions Bugzilla/Attachment.pm
Original file line number Diff line number Diff line change
Expand Up @@ -442,7 +442,6 @@ sub _check_content_type {
{
ThrowUserError("invalid_content_type", {contenttype => $content_type});
}
trick_taint($content_type);

return $content_type;
}
Expand Down Expand Up @@ -498,7 +497,6 @@ sub _check_filename {
# Truncate the filename to 100 characters, counting from the end of the
# string to make sure we keep the filename extension.
$filename = substr($filename, -100, 100);
trick_taint($filename);

return $filename;
}
Expand Down
3 changes: 0 additions & 3 deletions Bugzilla/Attachment/Database.pm
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@ use 5.10.1;
use strict;
use warnings;

use Bugzilla::Util qw(trick_taint);

sub new {
return bless({}, shift);
}
Expand All @@ -22,7 +20,6 @@ sub store {
my $dbh = Bugzilla->dbh;
my $sth = $dbh->prepare(
"INSERT INTO attach_data (id, thedata) VALUES ($attach_id, ?)");
trick_taint($data);
$sth->bind_param(1, $data, $dbh->BLOB_TYPE);
$sth->execute();
}
Expand Down
2 changes: 0 additions & 2 deletions Bugzilla/Auth/Login/Cookie.pm
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ sub get_login_info {
@{$cgi->{'Bugzilla_cookie_list'}};
$user_id = $cookie->value if $cookie;
}
trick_taint($login_cookie) if $login_cookie;
$self->cookie($login_cookie);

# If the call is for a web service, and an api token is provided, check
Expand Down Expand Up @@ -89,7 +88,6 @@ sub get_login_info {

# Anything goes for these params - they're just strings which
# we're going to verify against the db
trick_taint($login_cookie);
detaint_natural($user_id);

my $db_cookie = $dbh->selectrow_array(
Expand Down
2 changes: 0 additions & 2 deletions Bugzilla/Auth/Persist/Cookie.pm
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ sub persist_login {
= Bugzilla::Token::GenerateUniqueToken('logincookies', 'cookie');

my $ip_addr = remote_ip();
trick_taint($ip_addr);

$dbh->do('INSERT INTO logincookies (cookie, userid, ipaddr, lastused)
VALUES (?, ?, ?, NOW())', undef, $login_cookie, $user->id, $ip_addr);
Expand Down Expand Up @@ -144,7 +143,6 @@ sub logout {
# logged in and got the same cookie, we could be logging the other
# user out here. Yes, this is very very very unlikely, but why take
# chances? - bbaetz
map { trick_taint($_) } @login_cookies;
@login_cookies = map { $dbh->quote($_) } @login_cookies;
if ($type == LOGOUT_KEEP_CURRENT) {
$dbh->do(
Expand Down
4 changes: 0 additions & 4 deletions Bugzilla/Auth/Verify.pm
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ sub create_or_update_user {
my $username_user_id = login_to_id($username || '');
my $extern_user_id;
if ($extern_id) {
trick_taint($extern_id);
$extern_user_id = $dbh->selectrow_array(
'SELECT userid
FROM profiles WHERE extern_id = ?', undef, $extern_id
Expand Down Expand Up @@ -81,8 +80,6 @@ sub create_or_update_user {

# external authentication
# systems might follow different standards than ours. So in this
# place here, we call trick_taint without checks.
trick_taint($password);

# XXX Theoretically this could fail with an error, but the fix for
# that is too involved to be done right now.
Expand Down Expand Up @@ -133,7 +130,6 @@ sub create_or_update_user {

# $real_name is more than likely tainted, but we only use it
# in a placeholder and we never use it after this.
trick_taint($real_name);
$user->set_name($real_name);
$user_updated = 1;
}
Expand Down
6 changes: 0 additions & 6 deletions Bugzilla/Bug.pm
Original file line number Diff line number Diff line change
Expand Up @@ -2368,7 +2368,6 @@ sub _check_tag_name {
$tag = clean_text($tag);
$tag || ThrowUserError('no_tag_to_edit');
ThrowUserError('tag_name_too_long') if length($tag) > MAX_LEN_QUERY_NAME;
trick_taint($tag);

# Tags are all lowercase.
return lc($tag);
Expand Down Expand Up @@ -3451,7 +3450,6 @@ sub add_see_also {

my $field_values = $class->run_create_validators($params);
my $value = $field_values->{value}->as_string;
trick_taint($value);
$field_values->{value} = $value;

# We only add the new URI if it hasn't been added yet. URIs are
Expand Down Expand Up @@ -4345,7 +4343,6 @@ sub bug_alias_to_id {
my ($alias) = @_;
return undef unless Bugzilla->params->{"usebugaliases"};
my $dbh = Bugzilla->dbh;
trick_taint($alias);
return $dbh->selectrow_array("SELECT bug_id FROM bugs WHERE alias = ?",
undef, $alias);
}
Expand Down Expand Up @@ -4445,7 +4442,6 @@ sub GetBugActivity {
# Only consider changes since $starttime, if given.
my $datepart = "";
if (defined $starttime) {
trick_taint($starttime);
push(@args, $starttime);
$datepart = "AND bug_when > ?";
}
Expand Down Expand Up @@ -4674,8 +4670,6 @@ sub LogActivityEntry {
else {
$added = ""; # no more entries
}
trick_taint($addstr);
trick_taint($removestr);
my $fieldid = get_field_id($col);
$dbh->do(
"INSERT INTO bugs_activity
Expand Down
2 changes: 0 additions & 2 deletions Bugzilla/Comment.pm
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,6 @@ sub update {
$weighted->update();
}
}
trick_taint($tag);
$sth_delete->execute($self->id, $tag);
$sth_activity->execute($self->bug_id, $self->id, Bugzilla->user->id, $when, '',
$tag);
Expand All @@ -187,7 +186,6 @@ sub update {
else {
Bugzilla::Comment::TagWeights->create({tag => $tag, weight => 1});
}
trick_taint($tag);
$sth_insert->execute($self->id, $tag);
$sth_activity->execute($self->bug_id, $self->id, Bugzilla->user->id, $when,
$tag, '');
Expand Down
4 changes: 0 additions & 4 deletions Bugzilla/DB.pm
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,6 @@ use constant INDEX_DROPS_REQUIRE_FK_DROPS => 1;
sub quote {
my $self = shift;
my $retval = $self->dbh->quote(@_);
trick_taint($retval) if defined $retval;
return $retval;
}

Expand Down Expand Up @@ -474,9 +473,6 @@ sub sql_fulltext_search {
# in LIKE search clauses
@words = map($self->quote("\%$_\%"), @words);

# untaint words, since they are safe to use now that we've quoted them
trick_taint($_) foreach @words;

# turn the words into a set of LIKE search clauses
@words = map("LOWER($column) LIKE $_", @words);

Expand Down
3 changes: 0 additions & 3 deletions Bugzilla/DB/Mysql.pm
Original file line number Diff line number Diff line change
Expand Up @@ -181,9 +181,6 @@ sub sql_fulltext_search {
# quote the text for use in the MATCH AGAINST expression
$text = $self->quote($text);

# untaint the text, since it's safe to use now that we've quoted it
trick_taint($text);

return "MATCH($column) AGAINST($text $mode)";
}

Expand Down
1 change: 0 additions & 1 deletion Bugzilla/DB/Oracle.pm
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,6 @@ sub sql_from_days {
sub sql_fulltext_search {
my ($self, $column, $text) = @_;
$text = $self->quote($text);
trick_taint($text);
$fulltext_label++;
return "CONTAINS($column,$text,$fulltext_label) > 0", "SCORE($fulltext_label)";
}
Expand Down
1 change: 0 additions & 1 deletion Bugzilla/Elastic.pm
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ use 5.10.1;
use Moo;

use Bugzilla::Elastic::Search;
use Bugzilla::Util qw(trick_taint);

with 'Bugzilla::Elastic::Role::HasClient';

Expand Down
3 changes: 0 additions & 3 deletions Bugzilla/Elastic/Search.pm
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ use 5.10.1;
use Moo;
use Bugzilla::Search;
use Bugzilla::Search::Quicksearch;
use Bugzilla::Util qw(trick_taint);
use namespace::clean;

use Bugzilla::Elastic::Search::FakeCGI;
Expand Down Expand Up @@ -104,9 +103,7 @@ sub data {
$source->{relevance} = $hit->{_score};
foreach my $val (values %$source) {
next unless defined $val;
trick_taint($val);
}
trick_taint($hit->{_id});
if ($source) {
$hits{$hit->{_id}} = [@$source{@fields}];
}
Expand Down
3 changes: 0 additions & 3 deletions Bugzilla/Extension.pm
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ use Bugzilla::Install::Util qw( extension_code_files );

use File::Basename;
use File::Spec;
use Taint::Util qw(untaint);

BEGIN { push @INC, \&INC_HOOK }

Expand All @@ -35,7 +34,6 @@ sub INC_HOOK {
= Cwd::realpath(File::Spec->catpath($vol, File::Spec->catdir(@dirs), $file));

my $first = 1;
untaint($real_file);
$INC{$fake_file} = $real_file;
my $found = open my $fh, '<', $real_file;
unless ($found) {
Expand All @@ -48,7 +46,6 @@ sub INC_HOOK {
if (!$first) {
return 0 if eof $fh;
$_ = readline $fh or return 0;
untaint($_);
return 1;
}
else {
Expand Down
2 changes: 0 additions & 2 deletions Bugzilla/Field.pm
Original file line number Diff line number Diff line change
Expand Up @@ -1466,7 +1466,6 @@ sub check_field {
or !grep { $_ eq $value } @$legalsRef)
{
return 0 if $no_warn; # We don't want an error to be thrown; return.
trick_taint($name);

my $field = new Bugzilla::Field({name => $name});
my $field_desc = $field ? $field->description : $name;
Expand Down Expand Up @@ -1497,7 +1496,6 @@ sub get_field_id {
my ($name) = @_;
my $dbh = Bugzilla->dbh;

trick_taint($name);
my $id = $dbh->selectrow_array(
'SELECT id FROM fielddefs
WHERE name = ?', undef, $name
Expand Down
1 change: 0 additions & 1 deletion Bugzilla/Flag.pm
Original file line number Diff line number Diff line change
Expand Up @@ -965,7 +965,6 @@ sub extract_flags_from_cgi {
});

my $status = $cgi->param("flag_type-$type_id");
trick_taint($status);

my @logins = $cgi->param("requestee_type-$type_id");
if ($status eq "?" && scalar(@logins)) {
Expand Down
2 changes: 0 additions & 2 deletions Bugzilla/FlagType.pm
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,6 @@ sub _check_group {
my ($invocant, $group) = @_;
return unless $group;

trick_taint($group);
$group = Bugzilla::Group->check($group);
return $group->id;
}
Expand Down Expand Up @@ -682,7 +681,6 @@ sub sqlify_criteria {

if ($criteria->{name}) {
my $name = $dbh->quote($criteria->{name});
trick_taint($name); # Detaint data as we have quoted it.
push(@criteria, "flagtypes.name = $name");
}
if ($criteria->{target_type}) {
Expand Down
4 changes: 0 additions & 4 deletions Bugzilla/Install/Localconfig.pm
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ use List::Util qw(first);
use Tie::Hash::NamedCapture;
use Safe;
use Term::ANSIColor;
use Taint::Util qw(untaint);
use Sys::Hostname qw(hostname);

use parent qw(Exporter);
Expand Down Expand Up @@ -124,17 +123,14 @@ sub _read_localconfig_from_env {
foreach my $override (PARAM_OVERRIDE) {
my $o_key = ENV_PREFIX . $override;
$localconfig{param_override}{$override} = $ENV{$o_key};
untaint($localconfig{param_override}{$override});
}
}
elsif (exists $ENV{$key}) {
$localconfig{$name} = $ENV{$key};
untaint($localconfig{$name});
}
else {
my $default = $var->{default};
$localconfig{$name} = ref($default) eq 'CODE' ? $default->() : $default;
untaint($localconfig{$name});
}
}

Expand Down
10 changes: 0 additions & 10 deletions Bugzilla/Install/Util.pm
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,6 @@ sub extension_code_files {
# We know that these paths are safe, because they came from
# extensionsdir and we checked them specifically for their format.
# Also, the only thing we ever do with them is pass them to "require".
trick_taint($_) foreach @load_files;
push(@files, \@load_files);
}

Expand Down Expand Up @@ -308,7 +307,6 @@ sub _template_lang_directories {
foreach my $dir (@add) {
my $full_dir = "$templatedir/$lang/$dir";
if (-d $full_dir) {
trick_taint($full_dir);
push(@result, $full_dir);
}
}
Expand Down Expand Up @@ -593,14 +591,6 @@ use constant _cache => {};
# Copied from Bugzilla::Util #
##############################

sub trick_taint {
require Carp;
Carp::confess("Undef to trick_taint") unless defined $_[0];
my $match = $_[0] =~ /^(.*)$/s;
$_[0] = $match ? $1 : undef;
return (defined($_[0]));
}

sub trim {
my ($str) = @_;
if ($str) {
Expand Down
2 changes: 0 additions & 2 deletions Bugzilla/Logging.pm
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,11 @@ use Log::Log4perl::MDC;
use File::Spec::Functions qw(rel2abs catfile);
use Bugzilla::Constants qw(bz_locations);
use English qw(-no_match_vars $PROGRAM_NAME);
use Taint::Util qw(untaint);

sub logfile {
my ($class, $name) = @_;

my $file = rel2abs(catfile(bz_locations->{logsdir}, $name));
untaint($file);
return $file;
}

Expand Down
Loading

0 comments on commit 470728e

Please sign in to comment.