Bug#350954: DSA-960-1 security update breaks libmail-audit-perl when $ENV{HOME} is not set

[Robert L Mathews]

Package: libmail-audit-perl
Version: 2.1-5sarge2
Tags: patch

The libmail-audit-perl 2.1-5sarge2 NMU update tries to fix the insecure 
temporary file creation bug in Mail::Audit. However, the fix can cause 
serious problems in cases where $ENV{HOME} is not set.

The problem is with this line at the top:

   (undef,$logfile) = tempfile("mail_audit.log-XXXXX",TMPDIR=>1);

The first problem with this is that "TMPDIR=>1" is not a valid option 
for File::Temp's "tempfile" -- that option only works with tempdir(). So 
that option is being ignored and the temporary file is being created in 
the current working directory instead, which fails if the program 
doesn't have write permission for that directory. This is easily 
demonstrated as follows:

   $ cd /
   $ /usr/bin/env -i perl -e 'use Mail::Audit;'
   Error in tempfile() using mail_audit.log-XXXXX: Parent directory (.)
   is not writable at /usr/share/perl5/Mail/Audit.pm line 12

The correct usage to make tempfile() use a temporary directory would be 
something like:

   (undef,$logfile) =
       tempfile("mail_audit.log-XXXXX", DIR => File::Spec->tmpdir);

The second problem with having the "tempfile()" line at the top of the 
script when $ENV{HOME} is not present is that Mail::Audit now attempts 
to create a temporary log file every time it is used, even if logging is 
not enabled:

   $ cd /tmp
   $ /usr/bin/env -i perl -e 'use Mail::Audit;'

This creates an orphaned empty file in /tmp every time it's run.

The combination of these two problems is serious: in my case, a working 
script that uses Mail::Audit and doesn't do any logging (which was 
therefore not affected by the security bug in the first place) is now 
trying, and failing, to create a temporary log file in the current 
working directory every time the script is run, rendering the script 
unusable.

A proposed solution is attached. This patch only calls tempfile() if 
logging is actually used with a missing filename. And if logging is 
used, it uses "File::Spec->tmpdir" to locate "/tmp".

Although the code includes an extra "use File::Spec" for clarity, this 
is not actually a new dependency, because the code already uses 
File::Temp which depends on File::Spec anyway.

The patch also updates the documentation, which got out of sync when the 
behavior changed from the security update.

Thanks!

-- 
Robert L Mathews, Tiger Technologies    http://www.tigertech.net/

-------------- next part --------------
A non-text attachment was scrubbed...
Name: Audit.pm.patch
Type: text/x-patch
Size: 2408 bytes
Desc: not available
Url : http://lists.alioth.debian.org/pipermail/pkg-perl-maintainers/attachments/20060201/fd049c9f/Audit.pm.bin