Bug#344029: DSA-960 - New bug possibly introduced

[Brian Hodges]

Hello,

The recent security update for libmail-audit-perl (DSA-960) appears to 
have introduced a new bug.  I have been using debian for several years now 
and this is the first time that a security update turned out to be 
problematic for me.  Still an excellent track record in my book. :)

E-mail is often a touchy subject for a lot of people, so I thought I would 
post the problem I encountered, which might be causing delivery problems 
for other Debian/Mail::Audit users.

I am using Woody, Exim 3 and a perl script that make use of Mail::Audit. This
script executes as the mail user; the same user id under which Exim is running.

The problematic portion of the patch seems to be here:

-my $logfile = "/tmp/".getpwuid($>)."-audit.log";
+my $logfile;
+if (exists $ENV{HOME} and defined $ENV{HOME} and -d $ENV{HOME}) {
+     $logfile = "$ENV{HOME}/.mail_audit.log"
+}
+else {
+     (undef,$logfile) = tempfile("mail_audit.log-XXXXX",TMPDIR=>1);
+}

For reasons I haven't investigated, $ENV{HOME} is not being set when a 
child process (my script) is spawned.  This is causing the else clause to 
be triggered, in the above logic.  I further looked at the code for 
File::Temp, and don't see any reference to a 'TMPDIR' option related to 
the tempfile function.  I also have determined that the cwd of my 
executing script does not default to the mail user's home directory, but 
to an unwritable directory (/) under which $logfile cannot be written to.

So instead of relying on the HOME environment variable being set, it could
possibly make more sense to use to do a getpwuid call for the UID present in $<.

Below is a simple patch, but I'm sure there is more than one way to do it. I
didn't look in to how trustworthy $< is, but I think any serious risk is
mitigated with subsequent getpwuid call.

Thanks,

Brian Hodges

--- Audit.pm    Tue Jan 31 21:47:06 2006
+++ Audit-new.pm        Wed Feb  1 00:41:51 2006
@@ -6,17 +6,20 @@
use Sys::Hostname;
use vars qw($VERSION @ISA @EXPORT @EXPORT_OK);
use Fcntl ':flock';
-use File::Temp qw(tempfile);
use constant REJECTED => 100;
use constant DELIVERED => 0;
my $loglevel=3;
my $logging =0;
my $logfile;
-if (exists $ENV{HOME} and defined $ENV{HOME} and -d $ENV{HOME}) {
-     $logfile = "$ENV{HOME}/.mail_audit.log"
-}
-else {
-     (undef,$logfile) = tempfile("mail_audit.log-XXXXX",TMPDIR=>1);
+
+# Home directory is in the 8th position
+my $home = (getpwuid($<))[7];
+
+# If current user's homedirectory is writable, assign $logfile.
+# Otherwise if $logfile remains unassigned, code lower down will throw an unhandled
+# exception if logging is on, err die that is.
+if (defined $home and -w $home) {
+     $logfile = "$home/.mail_audit.log";
}

 $VERSION = '2.0';