Hello,

This was really a vulnerability which allowed running any perl code or
commands (even as root), for anyone able to write .cf files/rules.

The bug is mitigated in SpamAssassin 3.4.3, which properly taints
configuration strings, and results in Perl complaining and not running
Greylisting.pm at all.

I've made a proper patch which addresses both the vulnerability and 3.4.3
compatibility.

=====================================================================
--- Greylisting.pm.orig 2019-12-18 17:49:40.351383764 +0200
+++ Greylisting.pm      2019-12-18 22:30:03.745497552 +0200
@@ -21,6 +21,7 @@

 use strict;
 use Mail::SpamAssassin::Plugin;
+use Mail::SpamAssassin::Util qw(untaint_var);
 our @ISA = qw(Mail::SpamAssassin::Plugin);

 sub new
@@ -65,9 +66,25 @@

     Mail::SpamAssassin::Plugin::dbg("GREYLISTING: called function");

-    $optionhash  =~ s/;/,/g;
+    #$optionhash  =~ s/;/,/g;
     # This is safe, right? (users shouldn't be able to set it in their config)
-    %option=eval $optionhash;
+    #%option=eval $optionhash;
+
+    # ... no, evaling random strings is not safe!!!
+    # Ditch eval and parse hash string manually to maintain backwards 
compatibility
+    $optionhash =~ s/^\s*\(\s*//;
+    $optionhash =~ s/\s*\)\s*$//;
+    foreach my $opt (split(/\s*;\s*/, $optionhash)) {
+       my @vals = split(/\s*=>\s*/, $opt, 2);
+       next unless defined $vals[1];
+       # Sanitize away quotes and any unneeded characters, then untaint
+       foreach (@vals) {
+           s/[^\w\/-]//gs;
+           $_ = untaint_var($_);
+       }
+       $option{$vals[0]} = $vals[1];
+    }
+
     $self->{'rangreylisting'}=1;

     foreach my $reqoption (qw ( method greylistsecs dontgreylistthreshold
=====================================================================

Cheers,
Henrik

Reply via email to