diff options
-rw-r--r-- | globals.pl | 38 | ||||
-rwxr-xr-x | processmail | 40 |
2 files changed, 68 insertions, 10 deletions
diff --git a/globals.pl b/globals.pl index aa50355a0..4abb4a065 100644 --- a/globals.pl +++ b/globals.pl @@ -68,9 +68,12 @@ use DBI; use Date::Format; # For time2str(). use Date::Parse; # For str2time(). -# use Carp; # for confess +use Carp; # for confess use RelationSet; +# $ENV{PATH} is not taint safe +delete $ENV{PATH}; + # Contains the version string for the current running Bugzilla. $::param{'version'} = '2.13'; @@ -84,6 +87,13 @@ $::dbwritesallowed = 1; # Joe Robins, 7/5/00 $::superusergroupset = "9223372036854775807"; +sub die_with_dignity { + my ($err_msg) = @_; + print $err_msg; + confess($err_msg); +} +$::SIG{__DIE__} = \&die_with_dignity; + sub ConnectToDatabase { my ($useshadow) = (@_); if (!defined $::db) { @@ -649,6 +659,12 @@ sub DBID_to_real_or_loginname { sub DBID_to_name { my ($id) = (@_); + # $id should always be a positive integer + if ($id =~ m/^([1-9][0-9]*)$/) { + $id = $1; + } else { + $::cachedNameArray{$id} = "__UNKNOWN__"; + } if (!defined $::cachedNameArray{$id}) { PushGlobalSQLState(); SendSQL("select login_name from profiles where userid = $id"); @@ -668,10 +684,12 @@ sub DBname_to_id { SendSQL("select userid from profiles where login_name = @{[SqlQuote($name)]}"); my $r = FetchOneColumn(); PopGlobalSQLState(); - if (!defined $r || $r eq "") { + # $r should be a positive integer, this makes Taint mode happy + if (defined $r && $r =~ m/^([1-9][0-9]*)$/) { + return $1; + } else { return 0; } - return $r; } @@ -699,6 +717,18 @@ sub DBNameToIdAndCheck { exit(0); } +# Use detaint_string() 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 detaint_string { + my ($str) = @_; + $str =~ m/^(.*)$/s; + $str = $1; +} + # 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 @@ -965,6 +995,8 @@ sub SqlQuote { # } $str =~ s/([\\\'])/\\$1/g; $str =~ s/\0/\\0/g; + # If it's been SqlQuote()ed, then it's safe, so we tell -T that. + $str = detaint_string($str); return "'$str'"; } diff --git a/processmail b/processmail index de0f4c7fe..0fcdbbdde 100755 --- a/processmail +++ b/processmail @@ -1,4 +1,4 @@ -#!/usr/bonsaitools/bin/perl -w +#!/usr/bonsaitools/bin/perl -wT # -*- Mode: perl; indent-tabs-mode: nil -*- # # The contents of this file are subject to the Mozilla Public @@ -27,11 +27,19 @@ use diagnostics; use strict; +use lib "."; require "globals.pl"; use RelationSet; + +# Shut up misguided -w warnings about "used only once". +sub processmail_sillyness { + my $zz; + $zz = $::db; +} + $| = 1; umask(0); @@ -102,6 +110,10 @@ sub ProcessOneBug { $values{$i} = shift(@row); } my ($start, $end) = (@row); + # $start and $end are considered safe because users can't touch them + $start = detaint_string($start); + $end = detaint_string($end); + my $ccSet = new RelationSet(); $ccSet->mergeFromDB("SELECT who FROM cc WHERE bug_id = $id"); $values{'cc'} = $ccSet->toString(); @@ -471,22 +483,20 @@ sub filterEmailGroup ($$$) { foreach my $person (@emailList) { - my $userid; my $lastCount = @filteredList; if ( $person eq '' ) { next; } - SendSQL("SELECT userid FROM profiles WHERE login_name = " - . SqlQuote($person) ); + my $userid = DBname_to_id($person); - if ( !($userid = FetchSQLData()) ) { + if ( ! $userid ) { push(@filteredList,$person); next; } SendSQL("SELECT emailflags FROM profiles WHERE " . "userid = $userid" ); - + my ($userFlagString) = FetchSQLData(); # If the sender doesn't want email, exclude them from list @@ -622,6 +632,12 @@ sub NewProcessOnePerson ($$$$$$$$$$) { return; } + # Sanitize $values{'groupset'} + if ($values{'groupset'} =~ m/(\d+)/) { + $values{'groupset'} = $1; + } else { + $values{'groupset'} = 0; + } SendSQL("SELECT userid, groupset & $values{'groupset'} " . "FROM profiles WHERE login_name = " . SqlQuote($person)); my ($userid, $groupset) = (FetchSQLData()); @@ -706,6 +722,9 @@ sub NewProcessOnePerson ($$$$$$$$$$) { # Code starts here ConnectToDatabase(); +# Set Taint mode for the SQL +$::db->{Taint} = 1; +# ^^^ Taint mode is still a work in progress... GetVersionTable(); if (open(FID, "<data/nomail")) { @@ -762,7 +781,14 @@ if ($ARGV[0] eq "rescanall") { ProcessOneBug($ARGV[0]); } } else { - ProcessOneBug($ARGV[0]); + my $bugnum; + if ($ARGV[0] =~ m/^([1-9][0-9]*)$/) { + $bugnum = $1; + } else { + print "Error calling processmail (bug id is not an integer)<br>\n"; + exit; + } + ProcessOneBug($bugnum); } exit; |