Bug 6148 - sa-update fails: Insecure dependency in mkdir
Summary: sa-update fails: Insecure dependency in mkdir
Status: RESOLVED FIXED
Alias: None
Product: Spamassassin
Classification: Unclassified
Component: sa-update (show other bugs)
Version: 3.3.0
Hardware: Other All
: P1 critical
Target Milestone: 3.3.0
Assignee: SpamAssassin Developer Mailing List
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-07-06 19:48 UTC by Warren Togami
Modified: 2009-11-02 14:22 UTC (History)
2 users (show)


Attachment Type Modified Status Actions Submitter/CLA Status
A workaround for $1 getting tainted and spreading taint patch None Mark Martinec [HasCLA]
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Warren Togami 2009-07-06 19:48:17 UTC 
perl-5.10.0-69.fc11.x86_64

sa-update of spamassassin-3.3.0-alpha1 fails with the *.pre files from trunk.  If I delete and instead copy the *.pre files from 3.2.5, then sa-update succeeds.

[root@newcaprica ~]# sa-update
Insecure dependency in mkdir while running with -T switch at /usr/lib/perl5/5.10.0/File/Path.pm line 104.

[root@newcaprica ~]# rm /etc/mail/spamassassin/*.pre
rm: remove regular file `init.pre'? y
rm: remove regular file `v310.pre'? y
rm: remove regular file `v312.pre'? y
rm: remove regular file `v320.pre'? y
[root@newcaprica ~]# cp /tmp/Mail-SpamAssassin-3.2.5/rules/*.pre /etc/mail/spamassassin/
[root@newcaprica ~]# sa-update
[root@newcaprica ~]# ls -l /var/lib/spamassassin/3.003000/
total 8
drwxr-xr-x. 2 root root 4096 2009-07-06 19:18 updates_spamassassin_org
-rw-r--r--. 1 root root 2339 2009-07-06 19:18 updates_spamassassin_org.cf
Comment 1 Mark Martinec 2009-07-07 06:01:30 UTC 
(my yesterdays posting, just for documentation):

I've seen it last week, looks like an old Perl bug of a tainted $1
is rearing its head again. The following patch to File/Basename.pm
avoids the trouble:

--- Basename.pm~        2009-06-09 16:31:34.000000000 +0200
+++ Basename.pm 2009-06-27 15:49:49.000000000 +0200
@@ -332,4 +332,5 @@
     my $type = $Fileparse_fstype;

+    local $1;
     if ($type eq 'MacOS') {
         $_[0] =~ s/([^:]):\z/$1/s;

Here the $_[0] is NOT tainted, but $1 is, so the $_[0] gets tainted,
which leads to a failure in mkdir further on.
Comment 2 Mark Martinec 2009-07-07 06:02:58 UTC 
  Bug 6148: avoid $1 from getting tainted by fiddling with
  sub get_description_for_rule, and by localizing $1 throughout
  the sa-update. Use explicit untaining by untaint_var there
  instead of manually untainting variables.
Sending        lib/Mail/SpamAssassin/Conf.pm
Sending        lib/Mail/SpamAssassin/PerMsgStatus.pm
Sending        sa-update.raw
Committed revision 791820.
Comment 3 Justin Mason 2009-07-07 06:04:18 UTC 
painful, but +1.
Comment 4 Mark Martinec 2009-07-07 06:12:04 UTC 
(In reply to comment #0)
> sa-update of spamassassin-3.3.0-alpha1 fails with the *.pre files from trunk. 
> If I delete and instead copy the *.pre files from 3.2.5, then sa-update
> succeeds.

Btw, the difference there in the .pre files is that AWL plugin is
now commented out in v310.pre. Interestingly, uncommenting it
(enabling it again) mysteriously avoids the taint bug.

I'll attach the diff to make it easier for Warren to test it.
Comment 5 Mark Martinec 2009-07-07 06:14:08 UTC 
Created attachment 4477 [details]
A workaround for $1 getting tainted and spreading taint
Comment 6 Mark Martinec 2009-08-06 05:42:00 UTC 
The fix appears to avoid the perl bug. Closing.
Comment 7 Mark Martinec 2009-09-17 13:15:39 UTC 
Apparently the workaround does not help to avoid the perl bug.
Also Bug 6206.
Comment 8 Mark Martinec 2009-09-17 13:16:50 UTC 
*** Bug 6206 has been marked as a duplicate of this bug. ***
Comment 9 Warren Togami 2009-09-17 13:29:51 UTC 
(In reply to comment #7)
> Apparently the workaround does not help to avoid the perl bug.
> Also Bug 6206.

The patch for Basename.pm is applied to perl here in the case of Fedora 12.  Are you referring to a different perl bug?
Comment 10 Mark Martinec 2009-09-17 13:43:24 UTC 
> The patch for Basename.pm is applied to perl here in the case of Fedora 12. 
> Are you referring to a different perl bug?

Good. Although this change to Basename.pm is only a workaround,
the bug is in perl, the $1 should not be able to get tainted there.

Re-closing, this is not the same issue after all, sorry.
Comment 11 Yves Orton 2009-11-02 14:22:36 UTC 
Hi. Would it be possible to find out more about this ticket in a reply to:

http://rt.perl.org/rt3//Public/Bug/Display.html?id=67962

Ive looked at the bug report, and the patches proposed, and cannot understand what might possibly be wrong internally in perl. 

Can you reduce this to a simpler test case please?

Yves