--- spamd/spamd.raw.orig 2009-12-15 22:09:03.000000000 +0100 +++ spamd/spamd.raw 2009-12-15 22:15:34.000000000 +0100 @@ -88,6 +88,7 @@ use Mail::SpamAssassin::SubProcBackChannel; use Mail::SpamAssassin::SpamdForkScaling qw(:pfstates); use Mail::SpamAssassin::Logger qw(:DEFAULT log_message); +use Mail::SpamAssassin::Util qw(untaint_var); use Mail::SpamAssassin::Timeout; use Getopt::Long; @@ -2076,7 +2076,21 @@ my $prefsfrom = $username; # the one passed, NOT $opt{username} - if ($prefsfrom eq $suidto || $opt{'vpopmail'}) { - $userdir = $suiddir; # reuse the already-looked-up info + if ($prefsfrom eq $suidto) { + $userdir = $suiddir; # reuse the already-looked-up info, tainted + } elsif ( $opt{'vpopmail'} ) { + # + # If vpopmail config enabled then set $userdir to virtual homedir + # + my $username_untainted; + $username_untainted = + untaint_var($username) if $username =~ /^[-:,.=+A-Za-z0-9_\@~]+\z/; + my $vpopdir = $suiddir; # This should work with common vpopmail setups + $userdir = `$vpopdir/bin/vuserinfo -d \Q$username_untainted\E`; + if ($? == 0) { + chomp($userdir); + } else { + $userdir = handle_user_vpopmail($username_untainted,$vpopdir); + } } else { $userdir = (getpwnam($prefsfrom))[7]; } @@ -2095,30 +2108,47 @@ # call this anyway, regardless of --user-config, so that # signal_user_changed() is called - handle_user_set_user_prefs($userdir, $username); + handle_user_set_user_prefs(untaint_var($userdir), $username); } -sub handle_user_set_user_prefs { - my ($dir, $username) = @_; - - # If vpopmail config enabled then set $dir to virtual homedir +sub handle_user_vpopmail { # - if ( $opt{'vpopmail'} ) { - my $vpopdir = $dir; - $dir = `$vpopdir/bin/vuserinfo -d \Q$username\E`; - if ($? != 0) { - # - # If vuserinfo failed $username could be an alias - # - $dir = `$vpopdir/bin/valias \Q$username\E`; - if ($? == 0 && $dir !~ /.+ -> &/) { - $dir =~ s,.+ -> (/.+)/Maildir/,$1,; + # If vuserinfo failed $username could be an alias + # As the alias could be an alias itself we'll try to resolve it recursively + # Because we're mistrusting vpopmail we'll set off an alarm + # + my $username = shift; + my $vpopdir = shift; + my $userdir; + my $vpoptimeout = 5; + my $vptimer = Mail::SpamAssassin::Timeout->new({ secs => $vpoptimeout }); + + $vptimer->run(sub { + my $vpopusername = $username; + local $1; + do { + my $vpopusername_tainted = `$vpopdir/bin/valias \Q$vpopusername\E`; + no re 'taint'; + if ($vpopusername_tainted =~ /.+ -> &?(.+)/) { + $vpopusername = untaint_var($1); } else { - undef($dir); + die "failed to resolve vpopmail user/alias '$username' using vuserinfo/valias"; } - } - chomp($dir); + } until (($userdir = `$vpopdir/bin/vuserinfo -d \Q$vpopusername\E`) && $? == 0); + $userdir =~ s{.+ -> (/.+)/Maildir/}{$1}; + }); + + if ($vptimer->timed_out()) { + undef $userdir; + die "failed to resolve vpopmail user/alias '$username' in time ($vpoptimeout seconds)"; + } else { + chomp($userdir); } + return $userdir; +} + +sub handle_user_set_user_prefs { + my ($dir, $username) = @_; # don't do this if we weren't passed a directory if ($dir) { --- lib/Mail/SpamAssassin/Util.pm.orig 2008-06-10 11:20:22.000000000 +0200 +++ lib/Mail/SpamAssassin/Util.pm 2009-12-15 22:17:41.000000000 +0100 @@ -50,7 +50,7 @@ require Exporter; @ISA = qw(Exporter); -@EXPORT = qw(local_tz base64_decode); +@EXPORT = qw(local_tz base64_decode untaint_var); use Mail::SpamAssassin; use Mail::SpamAssassin::Util::RegistrarBoundaries;