Justin Mason wrote:
Would it be possible to incorporate some additional recursion? 2 levels
of recursion may be too little, in my opinion, and it'd just require
an additional "for" loop scope in the patch below to fix bug 4714 too.
Okay, not a problem, here we go.
First off two questions:
- Is it okay to set alarm()s in spamd.raw or am I at risk of resetting
other alarms?
- Can I safely untaint $username at handle_user_setuid_basic() without
further checking?
These are the updated changes:
Daniel Albers writes:
2536: vpopmail/qmail code neither warning- nor 100% taint-safe -- No change
~ - The proposed change is far too extensive to be included in spamd
imho. I would rather switch to a VPopmail implementation from CPAN. ->
wontfix
Well, the former implementation would have also raised taint mode
warnings, but it was never executed in handle_user_set_user_prefs() I guess.
The vpopmail part is taint-safe now, but that's mainly because of
unrelated changes that happened in between the bug report and now.
4714: Vpopmail handling of aliases fails -- partly fixed
~ - Resolving vpopmail aliases is supported with the attached patch,
although it doesn't do recursion, but only one additional try. Again I
don't think this should be done in spamd or spamassassin codebase at all.
The patch now attempts to recursively resolve aliases. It doesn't
iterate any certain number of times, but tries for $vpoptimeout = 5
seconds, though.
This is meant to deal with situations where
a) vpopmail commands have become slow/unresponsive, which could happen
due to various issues, like database backend issues
b) a larger number of aliases were chained
Cheers, Daniel
Index: spamd/spamd.raw
===================================================================
--- spamd/spamd.raw (revision 669697)
+++ spamd/spamd.raw (working copy)
@@ -2084,6 +2084,39 @@
if ($prefsfrom eq $suidto) {
$userdir = $suiddir; # reuse the already-looked-up info
+ } elsif ( $opt{'vpopmail'} ) {
+ #
+ # If vpopmail config enabled then set $userdir to virtual homedir
+ #
+ untaint_var(\$username);
+ my $vpopdir = $suiddir; # This should work with common vpopmail setups
+ $userdir = `$vpopdir/bin/vuserinfo -d \Q$username\E`;
+ if ($? != 0) {
+ #
+ # 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
+ #
+ eval {
+ my $vpoptimeout = 5;
+ my $vpopusername = $username;
+ local $SIG{ALRM} = sub { die "in time ($vpoptimeout seconds)" };
+ alarm $vpoptimeout;
+ do {
+ $vpopusername = `$vpopdir/bin/valias \Q$vpopusername\E`;
+ $vpopusername =~ s/.+ -> &?(.+)/$1/
+ or die "using vuserinfo/valias";
+ untaint_var(\$vpopusername);
+ } until (($userdir = `$vpopdir/bin/vuserinfo -d \Q$vpopusername\E`) && $? == 0);
+ alarm 0;
+ $userdir =~ s,.+ -> (/.+)/Maildir/,$1,;
+ };
+ if ($@) {
+ info("spamd: failed to resolve vpopmail user/alias '$username' [EMAIL PROTECTED]");
+ undef $userdir;
+ }
+ }
+ chomp($userdir) if defined($userdir);
} else {
$userdir = (getpwnam($prefsfrom))[7];
}
@@ -2108,25 +2141,6 @@
sub handle_user_set_user_prefs {
my ($dir, $username) = @_;
- # If vpopmail config enabled then set $dir to virtual homedir
- #
- 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,;
- } else {
- undef($dir);
- }
- }
- chomp($dir);
- }
-
# don't do this if we weren't passed a directory
if ($dir) {
my $cf_file = $dir . "/.spamassassin/user_prefs";
