You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@spamassassin.apache.org by mm...@apache.org on 2007/09/11 20:23:27 UTC

svn commit: r574659 - in /spamassassin/trunk/lib/Mail/SpamAssassin: Bayes.pm BayesStore/DBM.pm Conf.pm Conf/Parser.pm Dns.pm HTML.pm Plugin/BodyRuleBaseExtractor.pm Plugin/Check.pm Plugin/HTMLEval.pm Plugin/Hashcash.pm Plugin/MIMEHeader.pm Util.pm

Author: mmartinec
Date: Tue Sep 11 11:23:26 2007
New Revision: 574659

URL: http://svn.apache.org/viewvc?rev=574659&view=rev
Log:
Use untaint_var() for explicit untainting, see bug 5645

Modified:
    spamassassin/trunk/lib/Mail/SpamAssassin/Bayes.pm
    spamassassin/trunk/lib/Mail/SpamAssassin/BayesStore/DBM.pm
    spamassassin/trunk/lib/Mail/SpamAssassin/Conf.pm
    spamassassin/trunk/lib/Mail/SpamAssassin/Conf/Parser.pm
    spamassassin/trunk/lib/Mail/SpamAssassin/Dns.pm
    spamassassin/trunk/lib/Mail/SpamAssassin/HTML.pm
    spamassassin/trunk/lib/Mail/SpamAssassin/Plugin/BodyRuleBaseExtractor.pm
    spamassassin/trunk/lib/Mail/SpamAssassin/Plugin/Check.pm
    spamassassin/trunk/lib/Mail/SpamAssassin/Plugin/HTMLEval.pm
    spamassassin/trunk/lib/Mail/SpamAssassin/Plugin/Hashcash.pm
    spamassassin/trunk/lib/Mail/SpamAssassin/Plugin/MIMEHeader.pm
    spamassassin/trunk/lib/Mail/SpamAssassin/Util.pm

Modified: spamassassin/trunk/lib/Mail/SpamAssassin/Bayes.pm
URL: http://svn.apache.org/viewvc/spamassassin/trunk/lib/Mail/SpamAssassin/Bayes.pm?rev=574659&r1=574658&r2=574659&view=diff
==============================================================================
--- spamassassin/trunk/lib/Mail/SpamAssassin/Bayes.pm (original)
+++ spamassassin/trunk/lib/Mail/SpamAssassin/Bayes.pm Tue Sep 11 11:23:26 2007
@@ -237,6 +237,7 @@
 
   if ($self->{conf}->{bayes_store_module}) {
     my $module = $self->{conf}->{bayes_store_module};
+    $module = Mail::SpamAssassin::Util::untaint_var($module);  # good enough?
     my $store;
 
     eval '

Modified: spamassassin/trunk/lib/Mail/SpamAssassin/BayesStore/DBM.pm
URL: http://svn.apache.org/viewvc/spamassassin/trunk/lib/Mail/SpamAssassin/BayesStore/DBM.pm?rev=574659&r1=574658&r2=574659&view=diff
==============================================================================
--- spamassassin/trunk/lib/Mail/SpamAssassin/BayesStore/DBM.pm (original)
+++ spamassassin/trunk/lib/Mail/SpamAssassin/BayesStore/DBM.pm Tue Sep 11 11:23:26 2007
@@ -1437,7 +1437,7 @@
       die "bayes: unable to find bayes_toks and bayes_seen, stopping\n";
     }
     # untaint @files (already safe after grep)
-    @files = map { /(.*)/, $1 } @files;
+    Mail::SpamAssassin::Util::untaint_var(\@files);
  	 
     for (@files) {
       my $src = "$dir/$_";

Modified: spamassassin/trunk/lib/Mail/SpamAssassin/Conf.pm
URL: http://svn.apache.org/viewvc/spamassassin/trunk/lib/Mail/SpamAssassin/Conf.pm?rev=574659&r1=574658&r2=574659&view=diff
==============================================================================
--- spamassassin/trunk/lib/Mail/SpamAssassin/Conf.pm (original)
+++ spamassassin/trunk/lib/Mail/SpamAssassin/Conf.pm Tue Sep 11 11:23:26 2007
@@ -232,6 +232,7 @@
       # Figure out if we're doing relative scores, remove the parens if we are
       my $relative = 0;
       foreach (@scores) {
+        local ($1);
         if (s/^\((-?\d+(?:\.\d+)?)\)$/$1/) {
 	  $relative = 1;
 	}
@@ -719,6 +720,7 @@
     setting => 'add_header',
     code => sub {
       my ($self, $key, $value, $line) = @_;
+      local ($1,$2,$3);
       if ($value !~ /^(ham|spam|all)\s+([A-Za-z0-9_-]+)\s+(.*?)\s*$/) {
         return $INVALID_VALUE;
       }
@@ -766,6 +768,7 @@
     setting => 'remove_header',
     code => sub {
       my ($self, $key, $value, $line) = @_;
+      local ($1,$2);
       if ($value !~ /^(ham|spam|all)\s+([A-Za-z0-9_-]+)\s*$/) {
         return $INVALID_VALUE;
       }
@@ -1831,6 +1834,7 @@
       }
 
       # convert to qr// while including modifiers
+      local ($1,$2,$3);
       $value =~ /^m?(\W)(.*)(?:\1|>|}|\)|\])(.*?)$/;
       my $pattern = $2;
       $pattern = "(?".$3.")".$pattern if $3;
@@ -2061,6 +2065,7 @@
     is_priv => 1,
     code => sub {
       my ($self, $key, $value, $line) = @_;
+      local ($1,$2);
       if ($value =~ /^(\S+)\s+(?:rbl)?eval:(.*)$/) {
         my ($name, $fn) = ($1, $2);
 
@@ -2110,6 +2115,7 @@
     is_priv => 1,
     code => sub {
       my ($self, $key, $value, $line) = @_;
+      local ($1,$2);
       if ($value =~ /^(\S+)\s+eval:(.*)$/) {
         $self->{parser}->add_test ($1, $2, $TYPE_BODY_EVALS);
       }
@@ -2178,6 +2184,7 @@
     is_priv => 1,
     code => sub {
       my ($self, $key, $value, $line) = @_;
+      local ($1,$2);
       if ($value =~ /^(\S+)\s+eval:(.*)$/) {
         $self->{parser}->add_test ($1, $2, $TYPE_RAWBODY_EVALS);
       } else {
@@ -2211,6 +2218,7 @@
     is_priv => 1,
     code => sub {
       my ($self, $key, $value, $line) = @_;
+      local ($1,$2);
       if ($value =~ /^(\S+)\s+eval:(.*)$/) {
         $self->{parser}->add_test ($1, $2, $TYPE_FULL_EVALS);
       } else {
@@ -2405,6 +2413,7 @@
     code => sub {
       return unless defined($Mail::SpamAssassin::Conf::COLLECT_REGRESSION_TESTS);
       my ($self, $key, $value, $line) = @_;
+      local ($1,$2,$3);
       if ($value !~ /^(\S+)\s+(ok|fail)\s+(.*)$/) { return $INVALID_VALUE; }
       $self->{parser}->add_regression_test($1, $2, $3);
     }
@@ -2557,6 +2566,7 @@
     default => '',
     code => sub {
       my ($self, $key, $value, $line) = @_;
+      local ($1);
       if ($value !~ /^([_A-Za-z0-9:]+)$/) { return $INVALID_VALUE; }
       $self->{bayes_store_module} = $1;
     }
@@ -2795,13 +2805,18 @@
       if ($value eq '') {
         return $MISSING_REQUIRED_VALUE;
       }
+      my ($package, $path);
+      local ($1,$2);
       if ($value =~ /^(\S+)\s+(\S+)$/) {
-        $self->load_plugin ($1, $2);
-      } elsif ($value =~ /^(?:\S+)$/) {
-        $self->load_plugin ($value);
+        ($package, $path) = ($1, $2);
+      } elsif ($value =~ /^\S+$/) {
+        ($package, $path) = ($value, undef);
       } else {
 	return $INVALID_VALUE;
       }
+      # is blindly untainting safe?  it is no worse than before
+      $_ = Mail::SpamAssassin::Util::untaint_var($_)  for ($package,$path);
+      $self->load_plugin ($package, $path);
     }
   });
 
@@ -2820,13 +2835,18 @@
       if ($value eq '') {
         return $MISSING_REQUIRED_VALUE;
       }
+      my ($package, $path);
+      local ($1,$2);
       if ($value =~ /^(\S+)\s+(\S+)$/) {
-        $self->load_plugin ($1, $2, 1);
-      } elsif ($value =~ /^(?:\S+)$/) {
-        $self->load_plugin ($value, undef, 1);
+        ($package, $path) = ($1, $2);
+      } elsif ($value =~ /^\S+$/) {
+        ($package, $path) = ($value, undef);
       } else {
 	return $INVALID_VALUE;
       }
+      # is blindly untainting safe?  it is no worse than before
+      $_ = Mail::SpamAssassin::Util::untaint_var($_)  for ($package,$path);
+      $self->load_plugin ($package, $path, 1);
     }
   });
 
@@ -3460,7 +3480,11 @@
   if ($path) {
     $path = $self->{parser}->fix_path_relative_to_current_file($path);
   }
-  $self->{main}->{plugins}->load_plugin ($package, $path, $silent);
+  # it wouldn't hurt to do some checking on validity of $package
+  # and $path before untainting them
+  $self->{main}->{plugins}->load_plugin (
+    Mail::SpamAssassin::Util::untaint_var($package),  # safe???
+    $path, $silent);
 }
 
 sub load_plugin_succeeded {

Modified: spamassassin/trunk/lib/Mail/SpamAssassin/Conf/Parser.pm
URL: http://svn.apache.org/viewvc/spamassassin/trunk/lib/Mail/SpamAssassin/Conf/Parser.pm?rev=574659&r1=574658&r2=574659&view=diff
==============================================================================
--- spamassassin/trunk/lib/Mail/SpamAssassin/Conf/Parser.pm (original)
+++ spamassassin/trunk/lib/Mail/SpamAssassin/Conf/Parser.pm Tue Sep 11 11:23:26 2007
@@ -266,11 +266,11 @@
     $key = lc $key;
     # convert all dashes in setting name to underscores.
     $key =~ s/-/_/g;
-
-    # Do a better job untainting this info ...
     $value = '' unless defined($value);
-    $value =~ /^(.*)$/;
-    $value = $1;
+
+#   # Do a better job untainting this info ...
+#   # $value = Mail::SpamAssassin::Util::untaint_var($value);
+#   Do NOT blindly untaint now, do it carefully later when semantics is known!
 
     my $parse_error;       # undef by default, may be overridden
 
@@ -482,8 +482,8 @@
   my $eval = '';
   my $bad = 0;
   foreach my $token (@tokens) {
-    if ($token =~ /^(\W+|[+-]?\d+(?:\.\d+)?)$/) {
-      $eval .= $1." ";          # note: untaints!
+    if ($token =~ /^\W+|[+-]?\d+(?:\.\d+)?$/) {
+      $eval .= Mail::SpamAssassin::Util::untaint_var($token) . " ";
     }
     elsif ($token eq 'plugin') {
       # replace with method call
@@ -492,8 +492,8 @@
     elsif ($token eq 'version') {
       $eval .= $Mail::SpamAssassin::VERSION." ";
     }
-    elsif ($token =~ /^(\w[\w\:]+)$/) { # class name
-      $eval .= "\"$1\" ";       # note: untaints!
+    elsif ($token =~ /^\w[\w\:]+$/) { # class name
+      $eval .= '"' . Mail::SpamAssassin::Util::untaint_var($token) . '" ';
     }
     else {
       $bad++;
@@ -611,8 +611,8 @@
   unless ($value =~ /^-?\d+(?:\.\d+)?$/) {
     return $Mail::SpamAssassin::Conf::INVALID_VALUE;
   }
-
-  $conf->{$key} = $value+0.0;
+  # it is safe to untaint now that we now the syntax is a valid number
+  $conf->{$key} = Mail::SpamAssassin::Util::untaint_var($value) + 0.0;
 }
 
 sub set_bool_value {
@@ -624,18 +624,17 @@
 
   # bug 4462: allow yes/1 and no/0 for boolean values
   $value = lc $value;
-  if ($value eq 'yes') {
+  if ($value eq 'yes' || $value eq '1') {
     $value = 1;
   }
-  elsif ($value eq 'no') {
+  elsif ($value eq 'no' || $value eq '0') {
     $value = 0;
   }
-
-  unless ($value =~ /^[01]$/) {
+  else {
     return $Mail::SpamAssassin::Conf::INVALID_VALUE;
   }
 
-  $conf->{$key} = $value+0;
+  $conf->{$key} = $value;
 }
 
 sub set_string_value {
@@ -645,7 +644,7 @@
     return $Mail::SpamAssassin::Conf::MISSING_REQUIRED_VALUE;
   }
 
-  $conf->{$key} = $value;
+  $conf->{$key} = $value;  # keep tainted
 }
 
 sub set_hash_key_value {
@@ -656,7 +655,7 @@
     return $Mail::SpamAssassin::Conf::MISSING_REQUIRED_VALUE;
   }
 
-  $conf->{$key}->{$k} = $v;
+  $conf->{$key}->{$k} = $v;  # keep tainted
 }
 
 sub set_addrlist_value {
@@ -665,7 +664,7 @@
   unless (defined $value && $value !~ /^$/) {
     return $Mail::SpamAssassin::Conf::MISSING_REQUIRED_VALUE;
   }
-  $conf->{parser}->add_to_addrlist ($key, split (' ', $value));
+  $conf->{parser}->add_to_addrlist ($key, split (' ', $value));  # keep tainted
 }
 
 sub remove_addrlist_value {
@@ -680,7 +679,7 @@
 sub set_template_append {
   my ($conf, $key, $value, $line) = @_;
   if ( $value =~ /^"(.*?)"$/ ) { $value = $1; }
-  $conf->{$key} .= $value."\n";
+  $conf->{$key} .= $value."\n";  # keep tainted
 }
 
 sub set_template_clear {
@@ -832,7 +831,7 @@
     next unless exists $conf->{tests}->{$token};
 
     # add and recurse
-    push @{$deps}, $token;
+    push(@{$deps}, Mail::SpamAssassin::Util::untaint_var($token));
     $self->_meta_deps_recurse($conf, $toprule, $token, $deps, $alreadydone);
   }
 }
@@ -1091,6 +1090,7 @@
   my ($self, $name, $rule) = @_;
 
   my $meta = '';
+  $rule = Mail::SpamAssassin::Util::untaint_var($rule); # must be careful below
 
   # Lex the rule into tokens using a rather simple RE method ...
   my $lexer = ARITH_EXPRESSION_LEXER;
@@ -1141,6 +1141,7 @@
   my $origre = $re;
   my $safere = $re;
   my $mods = '';
+  local ($1,$2);
   if ($re =~ s/^m{//) {
     $re =~ s/}([a-z]*)$//; $mods = $1;
   }

Modified: spamassassin/trunk/lib/Mail/SpamAssassin/Dns.pm
URL: http://svn.apache.org/viewvc/spamassassin/trunk/lib/Mail/SpamAssassin/Dns.pm?rev=574659&r1=574658&r2=574659&view=diff
==============================================================================
--- spamassassin/trunk/lib/Mail/SpamAssassin/Dns.pm (original)
+++ spamassassin/trunk/lib/Mail/SpamAssassin/Dns.pm Tue Sep 11 11:23:26 2007
@@ -301,10 +301,8 @@
 	$subtest =~ s/\bS(\d+)\b/\$sb{$1}/;
       }
 
-      # untaint. doing the usual $subtest=$1 doesn't work! (bug 3325)
-      $subtest =~ /^(.*)$/;
-      my $untainted = $1;
-      $subtest = $untainted;
+      # untaint. (bug 3325)
+      $subtest = Mail::SpamAssassin::Util::untaint_var($subtest);
 
       $self->got_hit($rule, "SenderBase: ", ruletype => "dnsbl") if !$undef && eval $subtest;
     }

Modified: spamassassin/trunk/lib/Mail/SpamAssassin/HTML.pm
URL: http://svn.apache.org/viewvc/spamassassin/trunk/lib/Mail/SpamAssassin/HTML.pm?rev=574659&r1=574658&r2=574659&view=diff
==============================================================================
--- spamassassin/trunk/lib/Mail/SpamAssassin/HTML.pm (original)
+++ spamassassin/trunk/lib/Mail/SpamAssassin/HTML.pm Tue Sep 11 11:23:26 2007
@@ -224,7 +224,7 @@
   $self->{closed_extra} = 0;
   $self->{text} = [];		# rendered text
 
-  $self->{length} += $1 if (length($text) =~ m/^(\d+)$/);	# untaint
+  $self->{length} += Mail::SpamAssassin::Util::untaint_var(length($text));
 
   # NOTE: We *only* need to fix the rendering when we verify that it
   # differs from what people see in their MUA.  Testing is best done with

Modified: spamassassin/trunk/lib/Mail/SpamAssassin/Plugin/BodyRuleBaseExtractor.pm
URL: http://svn.apache.org/viewvc/spamassassin/trunk/lib/Mail/SpamAssassin/Plugin/BodyRuleBaseExtractor.pm?rev=574659&r1=574658&r2=574659&view=diff
==============================================================================
--- spamassassin/trunk/lib/Mail/SpamAssassin/Plugin/BodyRuleBaseExtractor.pm (original)
+++ spamassassin/trunk/lib/Mail/SpamAssassin/Plugin/BodyRuleBaseExtractor.pm Tue Sep 11 11:23:26 2007
@@ -974,8 +974,7 @@
   if (open(IN, "<".$cachefile)) {
     my $str = join("", <IN>);
     close IN;
-    $str =~ /^(.*)$/s;
-    my $untainted = $1;
+    my $untainted = Mail::SpamAssassin::Util::untaint_var($str);
 
     my $VAR1;                 # Data::Dumper
     if (eval $untainted) {

Modified: spamassassin/trunk/lib/Mail/SpamAssassin/Plugin/Check.pm
URL: http://svn.apache.org/viewvc/spamassassin/trunk/lib/Mail/SpamAssassin/Plugin/Check.pm?rev=574659&r1=574658&r2=574659&view=diff
==============================================================================
--- spamassassin/trunk/lib/Mail/SpamAssassin/Plugin/Check.pm (original)
+++ spamassassin/trunk/lib/Mail/SpamAssassin/Plugin/Check.pm Tue Sep 11 11:23:26 2007
@@ -322,7 +322,7 @@
     loop_body => sub
   {
     my ($self, $pms, $conf, $rulename, $rule, %opts) = @_;
-    my $token;
+    $rule = Mail::SpamAssassin::Util::untaint_var($rule);  # presumably checked
 
     # Lex the rule into tokens using a rather simple RE method ...
     my $lexer = ARITH_EXPRESSION_LEXER;
@@ -335,7 +335,7 @@
     $rule_deps{$rulename} = [ ];
 
     # Go through each token in the meta rule
-    foreach $token (@tokens) {
+    foreach my $token (@tokens) {
 
       # Numbers can't be rule names
       if ($token =~ /^(?:\W+|[+-]?\d+(?:\.\d+)?)$/) {
@@ -458,6 +458,7 @@
   {
     my ($self, $pms, $conf, $rulename, $rule, %opts) = @_;
     my $def = '';
+    $rule = Mail::SpamAssassin::Util::untaint_var($rule);  # presumably checked
     my ($hdrname, $testtype, $pat) =
         $rule =~ /^\s*(\S+)\s*(\=|\!)\~\s*(\S.*?\S)\s*$/;
 
@@ -567,6 +568,7 @@
     loop_body => sub
   {
     my ($self, $pms, $conf, $rulename, $pat, %opts) = @_;
+    $pat = Mail::SpamAssassin::Util::untaint_var($pat);  # presumably checked
     my $sub;
     if (($conf->{tflags}->{$rulename}||'') =~ /\bmultiple\b/)
     {
@@ -639,6 +641,7 @@
     loop_body => sub
   {
     my ($self, $pms, $conf, $rulename, $pat, %opts) = @_;
+    $pat = Mail::SpamAssassin::Util::untaint_var($pat);  # presumably checked
     my $sub;
     if (($conf->{tflags}->{$rulename}||'') =~ /\bmultiple\b/) {
       $loopid++;
@@ -706,6 +709,7 @@
     loop_body => sub
   {
     my ($self, $pms, $conf, $rulename, $pat, %opts) = @_;
+    $pat = Mail::SpamAssassin::Util::untaint_var($pat);  # presumably checked
     my $sub;
     if (($pms->{conf}->{tflags}->{$rulename}||'') =~ /\bmultiple\b/)
     {
@@ -783,6 +787,7 @@
                 loop_body => sub
   {
     my ($self, $pms, $conf, $rulename, $pat, %opts) = @_;
+    $pat = Mail::SpamAssassin::Util::untaint_var($pat);  # presumably checked
     $self->add_evalstr ('
       if ($scoresptr->{q{'.$rulename.'}}) {
         pos $$fullmsgref = 0;
@@ -896,6 +901,7 @@
       }
     }
  
+    $test = Mail::SpamAssassin::Util::untaint_var($test);  # presumably checked
     my ($function, $argstr) = ($test,'');
     if ($test =~ s/^([^,]+)(,.*)$//gs) {
       ($function, $argstr) = ($1,$2);
@@ -1014,9 +1020,10 @@
 
 sub hash_line_for_rule {
   my ($self, $pms, $rulename) = @_;
-  return "\n".'#line 1 "'.
-        $pms->{conf}->{source_file}->{$rulename}.
-        ', rule '.$rulename.',"';
+  return sprintf("\n#line 1 \"%s, rule %s,\"",
+                 Mail::SpamAssassin::Util::untaint_var(
+                   $pms->{conf}->{source_file}->{$rulename}),
+                 $rulename);
 }
 
 sub is_user_rule_sub {

Modified: spamassassin/trunk/lib/Mail/SpamAssassin/Plugin/HTMLEval.pm
URL: http://svn.apache.org/viewvc/spamassassin/trunk/lib/Mail/SpamAssassin/Plugin/HTMLEval.pm?rev=574659&r1=574658&r2=574659&view=diff
==============================================================================
--- spamassassin/trunk/lib/Mail/SpamAssassin/Plugin/HTMLEval.pm (original)
+++ spamassassin/trunk/lib/Mail/SpamAssassin/Plugin/HTMLEval.pm Tue Sep 11 11:23:26 2007
@@ -117,13 +117,15 @@
 
 sub html_eval {
   my ($self, $pms, undef, $test, $rawexpr) = @_;
-  $rawexpr =~ /^([\<\>\=\!\-\+ 0-9]+)$/; my $expr = $1;
-
+  my $expr;
+  if ($rawexpr =~ /^[\<\>\=\!\-\+ 0-9]+$/) {
+    $expr = Mail::SpamAssassin::Util::untaint_var($rawexpr);
+  }
   # workaround bug 3320: wierd perl bug where additional, very explicit
   # untainting into a new var is required.
   my $tainted = $pms->{html}{$test};
   return unless defined($tainted);
-  $tainted =~ /^(.*)$/; my $val = $1;
+  my $val = Mail::SpamAssassin::Util::untaint_var($tainted);
 
   # just use the value in $val, don't copy it needlessly
   return eval "\$val $expr";

Modified: spamassassin/trunk/lib/Mail/SpamAssassin/Plugin/Hashcash.pm
URL: http://svn.apache.org/viewvc/spamassassin/trunk/lib/Mail/SpamAssassin/Plugin/Hashcash.pm?rev=574659&r1=574658&r2=574659&view=diff
==============================================================================
--- spamassassin/trunk/lib/Mail/SpamAssassin/Plugin/Hashcash.pm (original)
+++ spamassassin/trunk/lib/Mail/SpamAssassin/Plugin/Hashcash.pm Tue Sep 11 11:23:26 2007
@@ -205,7 +205,9 @@
   $hc =~ s/\s+//gs;       # remove whitespace from multiline, folded tokens
 
   # untaint the string for paranoia, making sure not to allow \n \0 \' \"
-  $hc =~ /^([-A-Za-z0-9\xA0-\xFF:_\/\%\@\.\,\= \*\+\;]+)$/; $hc = $1;
+  if ($hc =~ /^[-A-Za-z0-9\xA0-\xFF:_\/\%\@\.\,\= \*\+\;]+$/) {
+    $hc = Mail::SpamAssassin::Util::untaint_var($hc);
+  }
   if (!$hc) { return 0; }
 
   my ($ver, $bits, $date, $rsrc, $exts, $rand, $trial);

Modified: spamassassin/trunk/lib/Mail/SpamAssassin/Plugin/MIMEHeader.pm
URL: http://svn.apache.org/viewvc/spamassassin/trunk/lib/Mail/SpamAssassin/Plugin/MIMEHeader.pm?rev=574659&r1=574658&r2=574659&view=diff
==============================================================================
--- spamassassin/trunk/lib/Mail/SpamAssassin/Plugin/MIMEHeader.pm (original)
+++ spamassassin/trunk/lib/Mail/SpamAssassin/Plugin/MIMEHeader.pm Tue Sep 11 11:23:26 2007
@@ -99,11 +99,13 @@
     is_priv => 1,
     code => sub {
       my ($self, $key, $value, $line) = @_;
+      local ($1,$2,$3,$4);
       if ($value !~ /^(\S+)\s+(\S+)\s*([\=\!]\~)\s*(.+)$/) {
         return $Mail::SpamAssassin::Conf::INVALID_VALUE;
       }
 
-      my $rulename = $1;
+      # provide stricter syntax for rule name!?
+      my $rulename = Mail::SpamAssassin::Util::untaint_var($1);
       my $hdrname = $2;
       my $negated = ($3 eq '!~') ? 1 : 0;
       my $pattern = $4;

Modified: spamassassin/trunk/lib/Mail/SpamAssassin/Util.pm
URL: http://svn.apache.org/viewvc/spamassassin/trunk/lib/Mail/SpamAssassin/Util.pm?rev=574659&r1=574658&r2=574659&view=diff
==============================================================================
--- spamassassin/trunk/lib/Mail/SpamAssassin/Util.pm (original)
+++ spamassassin/trunk/lib/Mail/SpamAssassin/Util.pm Tue Sep 11 11:23:26 2007
@@ -129,8 +129,11 @@
     foreach my $dir (File::Spec->path()) {
       next unless $dir;
 
-      $dir =~ /^(.+)$/; # untaint, then clean ( 'foo/./bar' -> 'foo/bar', etc. )
-      $dir = File::Spec->canonpath($1);
+      # untaint if at least 1 char and no NL (is the restriction intentional?)
+      local ($1);
+      $dir = untaint_var($1)  if $dir =~ /^(.+)$/;
+      # then clean ( 'foo/./bar' -> 'foo/bar', etc. )
+      $dir = File::Spec->canonpath($dir);
 
       if (!File::Spec->file_name_is_absolute($dir)) {
 	dbg("util: PATH included '$dir', which is not absolute, dropping");
@@ -212,15 +215,16 @@
   return unless defined($path);
   return '' if ($path eq '');
 
+  local ($1);
   # Barry Jaspan: allow ~ and spaces, good for Windows.  Also return ''
   # if input is '', as it is a safe path.
   my $chars = '-_A-Za-z\xA0-\xFF0-9\.\%\@\=\+\,\/\\\:';
   my $re = qr/^\s*([$chars][${chars}~ ]*)$/o;
 
   if ($path =~ $re) {
-    return $1;
+    return untaint_var($1);
   } else {
-    warn "util: cannot untaint path: \"$path\"\n";
+    warn "util: refusing to untaint suspicious path: \"$path\"\n";
     return $path;
   }
 }
@@ -236,8 +240,8 @@
   #   $domain = qq<$label(?:\.$label)*>;
   #   length($host) <= 255 && $host =~ /^($domain)$/
   # expanded (no variables in the re) because of a tainting bug in Perl 5.8.0
-  if (length($host) <= 255 && $host =~ /^([a-z\d](?:[a-z\d-]{0,61}[a-z\d])?(?:\.[a-z\d](?:[a-z\d-]{0,61}[a-z\d])?)*)$/i) {
-    return $1;
+  if (length($host) <= 255 && $host =~ /^[a-z\d](?:[a-z\d-]{0,61}[a-z\d])?(?:\.[a-z\d](?:[a-z\d-]{0,61}[a-z\d])?)*$/i) {
+    return untaint_var($host);
   }
   else {
     warn "util: cannot untaint hostname: \"$host\"\n";
@@ -955,8 +959,7 @@
 
 # thanks to http://www2.picante.com:81/~gtaylor/autobuse/ for this code
 sub secure_tmpfile {
-  my $tmpdir = Mail::SpamAssassin::Util::untaint_file_path(
-                $ENV{'TMPDIR'} || File::Spec->tmpdir());
+  my $tmpdir = untaint_file_path($ENV{'TMPDIR'} || File::Spec->tmpdir());
 
   if (!$tmpdir) {
     # Note: we would prefer to keep this fatal, as not being able to
@@ -1018,7 +1021,7 @@
 
 # stolen from secure_tmpfile()
 sub secure_tmpdir {
-  my $tmpdir = Mail::SpamAssassin::Util::untaint_file_path(File::Spec->tmpdir());
+  my $tmpdir = untaint_file_path(File::Spec->tmpdir());
 
   if (!$tmpdir) {
     # Note: we would prefer to keep this fatal, as not being able to