diff options
| -rwxr-xr-x | Bug.pm | 6 | ||||
| -rwxr-xr-x | Bugzilla/Bug.pm | 6 | ||||
| -rw-r--r-- | Bugzilla/Search.pm | 40 | ||||
| -rw-r--r-- | Bugzilla/Util.pm | 260 | ||||
| -rw-r--r-- | CGI.pl | 27 | ||||
| -rwxr-xr-x | checksetup.pl | 1 | ||||
| -rw-r--r-- | globals.pl | 75 |
7 files changed, 294 insertions, 121 deletions
@@ -33,6 +33,8 @@ package Bug; use CGI::Carp qw(fatalsToBrowser); my %ok_field; +use Bugzilla::Util; + for my $key (qw (bug_id alias product version rep_platform op_sys bug_status resolution priority bug_severity component assigned_to reporter bug_file_loc short_desc target_milestone @@ -78,7 +80,7 @@ sub initBug { $bug_id = &::BugAliasToID($bug_id) if $bug_id !~ /^[1-9][0-9]*$/; my $old_bug_id = $bug_id; - if ((! defined $bug_id) || (!$bug_id) || (!&::detaint_natural($bug_id))) { + if ((! defined $bug_id) || (!$bug_id) || (!detaint_natural($bug_id))) { # no bug number given or the alias didn't match a bug $self->{'bug_id'} = $old_bug_id; $self->{'error'} = "InvalidBugId"; @@ -387,7 +389,7 @@ sub CanChangeField { if ($oldvalue eq $newvalue) { return 1; } - if (&::trim($oldvalue) eq &::trim($newvalue)) { + if (trim($oldvalue) eq trim($newvalue)) { return 1; } if ($f =~ /^longdesc/) { diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm index d73bc536f..51b51cec9 100755 --- a/Bugzilla/Bug.pm +++ b/Bugzilla/Bug.pm @@ -33,6 +33,8 @@ package Bug; use CGI::Carp qw(fatalsToBrowser); my %ok_field; +use Bugzilla::Util; + for my $key (qw (bug_id alias product version rep_platform op_sys bug_status resolution priority bug_severity component assigned_to reporter bug_file_loc short_desc target_milestone @@ -78,7 +80,7 @@ sub initBug { $bug_id = &::BugAliasToID($bug_id) if $bug_id !~ /^[1-9][0-9]*$/; my $old_bug_id = $bug_id; - if ((! defined $bug_id) || (!$bug_id) || (!&::detaint_natural($bug_id))) { + if ((! defined $bug_id) || (!$bug_id) || (!detaint_natural($bug_id))) { # no bug number given or the alias didn't match a bug $self->{'bug_id'} = $old_bug_id; $self->{'error'} = "InvalidBugId"; @@ -387,7 +389,7 @@ sub CanChangeField { if ($oldvalue eq $newvalue) { return 1; } - if (&::trim($oldvalue) eq &::trim($newvalue)) { + if (trim($oldvalue) eq trim($newvalue)) { return 1; } if ($f =~ /^longdesc/) { diff --git a/Bugzilla/Search.pm b/Bugzilla/Search.pm index e73c6660e..257b7656d 100644 --- a/Bugzilla/Search.pm +++ b/Bugzilla/Search.pm @@ -35,6 +35,8 @@ use vars qw($userid $usergroupset); package Bugzilla::Search; +use Bugzilla::Util; + # Create a new Search sub new { my $invocant = shift; @@ -66,33 +68,33 @@ sub init { my @andlist; # First, deal with all the old hard-coded non-chart-based poop. - if (&::lsearch($fieldsref, 'map_assigned_to.login_name') >= 0) { + if (lsearch($fieldsref, 'map_assigned_to.login_name') >= 0) { push @supptables, "profiles AS map_assigned_to"; push @wherepart, "bugs.assigned_to = map_assigned_to.userid"; } - if (&::lsearch($fieldsref, 'map_reporter.login_name') >= 0) { + if (lsearch($fieldsref, 'map_reporter.login_name') >= 0) { push @supptables, "profiles AS map_reporter"; push @wherepart, "bugs.reporter = map_reporter.userid"; } - if (&::lsearch($fieldsref, 'map_qa_contact.login_name') >= 0) { + if (lsearch($fieldsref, 'map_qa_contact.login_name') >= 0) { push @supptables, "LEFT JOIN profiles map_qa_contact ON bugs.qa_contact = map_qa_contact.userid"; } - if (&::lsearch($fieldsref, 'map_products.name') >= 0) { + if (lsearch($fieldsref, 'map_products.name') >= 0) { push @supptables, "products AS map_products"; push @wherepart, "bugs.product_id = map_products.id"; } - if (&::lsearch($fieldsref, 'map_components.name') >= 0) { + if (lsearch($fieldsref, 'map_components.name') >= 0) { push @supptables, "components AS map_components"; push @wherepart, "bugs.component_id = map_components.id"; } my $minvotes; if (defined $F{'votes'}) { - my $c = &::trim($F{'votes'}); + my $c = trim($F{'votes'}); if ($c ne "") { if ($c !~ /^[0-9]*$/) { $::vars->{'value'} = $c; @@ -116,7 +118,7 @@ sub init { "target_milestone", "groupset"); foreach my $field (keys %F) { - if (&::lsearch(\@legal_fields, $field) != -1) { + if (lsearch(\@legal_fields, $field) != -1) { push(@specialchart, [$field, "anyexact", join(',', @{$M{$field}})]); } @@ -146,7 +148,7 @@ sub init { if (!defined ($F{"email$id"})) { next; } - my $email = &::trim($F{"email$id"}); + my $email = trim($F{"email$id"}); if ($email eq "") { next; } @@ -154,7 +156,7 @@ sub init { if ($type eq "exact") { $type = "anyexact"; foreach my $name (split(',', $email)) { - $name = &::trim($name); + $name = trim($name); if ($name) { &::DBNameToIdAndCheck($name); } @@ -186,7 +188,7 @@ sub init { if (defined $F{'changedin'}) { - my $c = &::trim($F{'changedin'}); + my $c = trim($F{'changedin'}); if ($c ne "") { if ($c !~ /^[0-9]*$/) { $::vars->{'value'} = $c; @@ -200,14 +202,14 @@ sub init { my $ref = $M{'chfield'}; if (defined $ref) { - my $which = &::lsearch($ref, "[Bug creation]"); + my $which = lsearch($ref, "[Bug creation]"); if ($which >= 0) { splice(@$ref, $which, 1); push(@specialchart, ["creation_ts", "greaterthan", SqlifyDate($F{'chfieldfrom'})]); my $to = $F{'chfieldto'}; if (defined $to) { - $to = &::trim($to); + $to = trim($to); if ($to ne "" && $to !~ /^now$/i) { push(@specialchart, ["creation_ts", "lessthan", SqlifyDate($to)]); @@ -229,7 +231,7 @@ sub init { &::SqlQuote(SqlifyDate($F{'chfieldfrom'}))); my $to = $F{'chfieldto'}; if (defined $to) { - $to = &::trim($to); + $to = trim($to); if ($to ne "" && $to !~ /^now$/i) { push(@wherepart, "actcheck.bug_when <= " . &::SqlQuote(SqlifyDate($to))); @@ -237,7 +239,7 @@ sub init { } my $value = $F{'chfieldvalue'}; if (defined $value) { - $value = &::trim($value); + $value = trim($value); if ($value ne "") { push(@wherepart, "actcheck.added = " . &::SqlQuote($value)) @@ -248,7 +250,7 @@ sub init { foreach my $f ("short_desc", "long_desc", "bug_file_loc", "status_whiteboard") { if (defined $F{$f}) { - my $s = &::trim($F{$f}); + my $s = trim($F{$f}); if ($s ne "") { my $n = $f; my $q = &::SqlQuote($s); @@ -731,14 +733,14 @@ sub init { $t = $F{"type$chart-$row-$col"} || "noop"; $v = $F{"value$chart-$row-$col"}; $v = "" if !defined $v; - $v = &::trim($v); + $v = trim($v); if ($f eq "noop" || $t eq "noop" || $v eq "") { next; } # chart -1 is generated by other code above, not from the user- # submitted form, so we'll blindly accept any values in chart -1 if ((!$chartfields{$f}) && ($chart != -1)) { - my $errstr = "Can't use " . &::html_quote($f) . " as a field name. " . + my $errstr = "Can't use " . html_quote($f) . " as a field name. " . "If you think you're getting this in error, please copy the " . "entire URL out of the address bar at the top of your browser " . "window and email it to <109679\@bugzilla.org>"; @@ -749,7 +751,7 @@ sub init { # This is either from the internal chart (in which case we # already know about it), or it was in %chartfields, so it is # a valid field name, which means that its ok. - &::trick_taint($f); + trick_taint($f); $q = &::SqlQuote($v); my $func; $term = undef; @@ -805,7 +807,7 @@ sub init { $query = &::SelectVisible($query, $::userid, $::usergroupset); if ($debug) { - print "<p><code>" . &::value_quote($query) . "</code></p>\n"; + print "<p><code>" . value_quote($query) . "</code></p>\n"; exit; } diff --git a/Bugzilla/Util.pm b/Bugzilla/Util.pm new file mode 100644 index 000000000..aabaabb88 --- /dev/null +++ b/Bugzilla/Util.pm @@ -0,0 +1,260 @@ +# -*- Mode: perl; indent-tabs-mode: nil -*- +# +# The contents of this file are subject to the Mozilla Public +# License Version 1.1 (the "License"); you may not use this file +# except in compliance with the License. You may obtain a copy of +# the License at http://www.mozilla.org/MPL/ +# +# Software distributed under the License is distributed on an "AS +# IS" basis, WITHOUT WARRANTY OF ANY KIND, either express or +# implied. See the License for the specific language governing +# rights and limitations under the License. +# +# The Original Code is the Bugzilla Bug Tracking System. +# +# The Initial Developer of the Original Code is Netscape Communications +# Corporation. Portions created by Netscape are +# Copyright (C) 1998 Netscape Communications Corporation. All +# Rights Reserved. +# +# Contributor(s): Terry Weissman <terry@mozilla.org> +# Dan Mosedale <dmose@mozilla.org> +# Jake <jake@acutex.net> +# Bradley Baetz <bbaetz@student.usyd.edu.au> +# Christopher Aillon <christopher@aillon.com> + +package Bugzilla::Util; + +=head1 NAME + +Bugzilla::Util - Generic utility functions for bugzilla + +=head1 SYNOPSIS + + use Bugzilla::Util; + + # Functions for dealing with variable tainting + $rv = is_tainted($var); + trick_taint($var); + detaint_natural($var); + + # Functions for quoting + html_quote($var); + value_quote($var); + + # Functions for searching + $loc = lsearch(\@arr, $val); + $val = max($a, $b, $c); + $val = min($a, $b, $c); + + # Functions for trimming variables + $val = trim(" abc "); + +=head1 DESCRIPTION + +This package contains various utility functions which do not belong anywhere +else. + +B<It is not intended as a general dumping group for something which +people feel might be useful somewhere, someday>. Do not add methods to this +package unless it is intended to be used for a significant number of files, +and it does not belong anywhere else. + +=cut + +use base qw(Exporter); +@Bugzilla::Util::EXPORT = qw(is_tainted trick_taint detaint_natural + html_quote value_quote + lsearch max min + trim); + +use strict; +use diagnostics; + +=head1 FUNCTIONS + +This package provides several types of routines: + +=head2 Tainting + +Several functions are available to deal with tainted variables. B<Use these +with care> to avoid security holes. + +=over 4 + +=item C<is_tainted> + +Determines whether a particular variable is tainted + +=cut + +# This is from the perlsec page, slightly modifed to remove a warning +# From that page: +# This function makes use of the fact that the presence of +# tainted data anywhere within an expression renders the +# entire expression tainted. +# Don't ask me how it works... +sub is_tainted { + return not eval { my $foo = join('',@_), kill 0; 1; }; +} + +=item C<trick_taint($val)> + +Tricks perl into untainting a particular variable. + +Use trick_taint() when you know that there is no way that the data +in a scalar can be tainted, but taint mode still bails on it. + +B<WARNING!! Using this routine on data that really could be tainted defeats +the purpose of taint mode. It should only be used on variables that have been sanity checked in some way and have been determined to be OK.> + +=cut + +sub trick_taint { + $_[0] =~ /^(.*)$/s; + $_[0] = $1; + return (defined($_[0])); +} + +=item C<detaint_natural($num)> + +This routine detaints a natural number. It returns a true value if the +value passed in was a valid natural number, else it returns false. You +B<MUST> check the result of this routine to avoid security holes. + +=cut + +sub detaint_natural { + $_[0] =~ /^(\d+)$/; + $_[0] = $1; + return (defined($_[0])); +} + +=back + +=head2 Quoting + +Some values may need to be quoted from perl. However, this should in general +be done in the template where possible. + +=over 4 + +=item C<html_quote($val)> + +Returns a value quoted for use in HTML, with &, E<lt>, E<gt>, and E<34> being +replaced with their appropriate HTML entities. + +=cut + +sub html_quote { + my ($var) = (@_); + $var =~ s/\&/\&/g; + $var =~ s/</\</g; + $var =~ s/>/\>/g; + $var =~ s/\"/\"/g; + return $var; +} + +=item C<value_quote($val)> + +As well as escaping html like C<html_quote>, this routine converts newlines +into 
, suitable for use in html attributes. + +=cut + +sub value_quote { + my ($var) = (@_); + $var =~ s/\&/\&/g; + $var =~ s/</\</g; + $var =~ s/>/\>/g; + $var =~ s/\"/\"/g; + # See bug http://bugzilla.mozilla.org/show_bug.cgi?id=4928 for + # explanaion of why bugzilla does this linebreak substitution. + # This caused form submission problems in mozilla (bug 22983, 32000). + $var =~ s/\r\n/\
/g; + $var =~ s/\n\r/\
/g; + $var =~ s/\r/\
/g; + $var =~ s/\n/\
/g; + return $var; +} + +=back + +=head2 Searching + +Functions for searching within a set of values. + +=over 4 + +=item C<lsearch($list, $item)> + +Returns the position of C<$item> in C<$list>. C<$list> must be a list +reference. + +If the item is not in the list, returns -1. + +=cut + +sub lsearch { + my ($list,$item) = (@_); + my $count = 0; + foreach my $i (@$list) { + if ($i eq $item) { + return $count; + } + $count++; + } + return -1; +} + +=item C<max($a, $b, ...)> + +Returns the maximum from a set of values. + +=cut + +sub max { + my $max = shift(@_); + foreach my $val (@_) { + $max = $val if $val > $max; + } + return $max; +} + +=item C<min($a, $b, ...)> + +Returns the minimum from a set of values. + +=cut + +sub min { + my $min = shift(@_); + foreach my $val (@_) { + $min = $val if $val < $min; + } + return $min; +} + +=back + +=head2 Trimming + +=over 4 + +=item C<trim($str)> + +Removes any leading or trailing whitespace from a string. This routine does not +modify the existing string. + +=cut + +sub trim { + my ($str) = @_; + $str =~ s/^\s+//g; + $str =~ s/\s+$//g; + return $str; +} + +=back + +=cut @@ -33,6 +33,8 @@ use lib "."; # use Carp; # for confess +use Bugzilla::Util; + # commented out the following snippet of code. this tosses errors into the # CGI if you are perl 5.6, and doesn't if you have perl 5.003. # We want to check for the existence of the LDAP modules here. @@ -334,31 +336,6 @@ sub ValidateComment { } } -sub html_quote { - my ($var) = (@_); - $var =~ s/\&/\&/g; - $var =~ s/</\</g; - $var =~ s/>/\>/g; - $var =~ s/"/\"/g; - return $var; -} - -sub value_quote { - my ($var) = (@_); - $var =~ s/\&/\&/g; - $var =~ s/</\</g; - $var =~ s/>/\>/g; - $var =~ s/"/\"/g; - # See bug http://bugzilla.mozilla.org/show_bug.cgi?id=4928 for - # explanaion of why bugzilla does this linebreak substitution. - # This caused form submission problems in mozilla (bug 22983, 32000). - $var =~ s/\r\n/\
/g; - $var =~ s/\n\r/\
/g; - $var =~ s/\r/\
/g; - $var =~ s/\n/\
/g; - return $var; -} - # Adds <link> elements for bug lists. These can be inserted into the header by # using the "header_html" parameter to PutHeader, which inserts an arbitrary # string into the header. This function is currently used only in diff --git a/checksetup.pl b/checksetup.pl index 728db3798..34b487443 100755 --- a/checksetup.pl +++ b/checksetup.pl @@ -912,7 +912,6 @@ END { strike => sub { return $_; } , js => sub { return $_; }, - html => sub { return $_; }, html_linebreak => sub { return $_; }, url_quote => sub { return $_; }, }, diff --git a/globals.pl b/globals.pl index 100c3ae2f..9f15976b2 100644 --- a/globals.pl +++ b/globals.pl @@ -28,6 +28,8 @@ use diagnostics; use strict; +use Bugzilla::Util; + # Shut up misguided -w warnings about "used only once". For some reason, # "use vars" chokes on me when I try it here. @@ -230,16 +232,6 @@ sub SqlLog { } } -# This is from the perlsec page, slightly modifed to remove a warning -# From that page: -# This function makes use of the fact that the presence of -# tainted data anywhere within an expression renders the -# entire expression tainted. -# Don't ask me how it works... -sub is_tainted { - return not eval { my $foo = join('',@_), kill 0; 1; }; -} - sub SendSQL { my ($str, $dontshadow) = (@_); @@ -353,21 +345,6 @@ sub GetFieldID { die "Unknown field id: $f" if !$fieldid; return $fieldid; } - - - - -sub lsearch { - my ($list,$item) = (@_); - my $count = 0; - foreach my $i (@$list) { - if ($i eq $item) { - return $count; - } - $count++; - } - return -1; -} # Generate a string which, when later interpreted by the Perl compiler, will # be the same as the given string. @@ -993,24 +970,6 @@ sub get_component_name { return $comp; } -# Use trick_taint() when you know that there is no way that the data -# in a scalar can be tainted, but taint mode still bails on it. -# WARNING!! Using this routine on data that really could be tainted -# defeats the purpose of taint mode. It should only be -# used on variables that cannot be touched by users. - -sub trick_taint { - $_[0] =~ /^(.*)$/s; - $_[0] = $1; - return (defined($_[0])); -} - -sub detaint_natural { - $_[0] =~ /^(\d+)$/; - $_[0] = $1; - return (defined($_[0])); -} - # This routine quoteUrls contains inspirations from the HTML::FromText CPAN # module by Gareth Rees <garethr@cre.canon.co.uk>. It has been heavily hacked, # all that is really recognizable from the original is bits of the regular @@ -1541,32 +1500,6 @@ sub PerformSubsts { return $str; } -# Min and max routines. -sub min { - my $min = shift(@_); - foreach my $val (@_) { - $min = $val if $val < $min; - } - return $min; -} - -sub max { - my $max = shift(@_); - foreach my $val (@_) { - $max = $val if $val > $max; - } - return $max; -} - -# Trim whitespace from front and back. - -sub trim { - my ($str) = @_; - $str =~ s/^\s+//g; - $str =~ s/\s+$//g; - return $str; -} - ############################################################################### # Global Templatization Code @@ -1614,8 +1547,6 @@ $::template ||= Template->new( $var =~ s/\r/\\r/g; return $var; } , - - html => \&html_quote , # HTML collapses newlines in element attributes to a single space, # so form elements which may have whitespace (ie comments) need @@ -1821,7 +1752,7 @@ $::vars = 'PerformSubsts' => \&PerformSubsts , # Generic linear search function - 'lsearch' => \&lsearch , + 'lsearch' => \&Bugzilla::Util::lsearch , # UserInGroup - you probably want to cache this 'UserInGroup' => \&UserInGroup , |