Bug #61930 for Archive-Zip: Taint problem in Archive::Zip::Member

Wed Oct 06 08:47:49 2010 http://www.google.com/profiles/eprenders - Ticket created

Subject:

Taint problem in Archive::Zip::Member

Programs calling Archive::Zip::Member when under -T taint mode get an insecure dependency error. I've replicated this by modifying t/10_chmod.t's shebang to read: #!/usr/bin/env perl -T Then prove -l t/10-chmod.t produces: t/10_chmod.t .. Insecure dependency in chmod while running with -T switch at /usr/local/src/Archive-Zip-1.30/lib/Archive/Zip/Member.pm line 490. There's another ticket, #42035, which reports this issue as being in Strawberry perl, but it's also happening on my platform which is perl 5.12.2 on Linux server18.netring.co.uk 2.6.18-194.17.1.el5. This is a custom perl built from source and compiled int /opt/perl5/. I'm more than happy to help resolving this issue - I've spent some time on it but I haven't figured it out yet. The error producing bit is: chmod ($self->unixFileAttributes(), $name) or return _error("Can't chmod() ${name}: $!"); It seems to be $self->unixFileAttributes() which is causing the taint error, rather than $name. I've tried to see where that's getting set from something tainted but haven't been able to spot it.

Thu Oct 14 13:37:51 2010 http://www.google.com/profiles/eprenders - Correspondence added

From:

info [...] madduck.co.uk

On Wed Oct 06 08:47:49 2010, http://www.google.com/profiles/eprenders wrote: Show quoted text

> Programs calling Archive::Zip::Member when under -T taint mode get an > insecure dependency error.

I've attached a proposed solution (thanks to tm604 on irc.freenode.org).

Subject:

patch.txt

diff --git a/lib/Archive/Zip/Member.pm b/lib/Archive/Zip/Member.pm index f86ef75..4bb2171 100644 --- a/lib/Archive/Zip/Member.pm +++ b/lib/Archive/Zip/Member.pm @@ -282,7 +282,7 @@ sub _mapPermissionsToUnix { if ( $format == FA_AMIGA ) { $attribs = $attribs >> 17 & 7; # Amiga RWE bits $mode = $attribs << 6 | $attribs << 3 | $attribs; - return $mode; + return sprintf("%d", $mode); } if ( $format == FA_THEOS ) { @@ -304,7 +304,10 @@ sub _mapPermissionsToUnix { || $format == FA_TANDEM ) { $mode = $attribs >> 16; - return $mode if $mode != 0 or not $self->localExtraField; + if( $mode != 0 or not $self->localExtraField) { + $mode = sprintf("%d", $mode); + return sprintf("%d", $mode); + } # warn("local extra field is: ", $self->localExtraField, "\n"); @@ -340,9 +343,13 @@ sub _mapPermissionsToUnix { # keep previous $mode setting when its "owner" # part appears to be consistent with DOS attribute flags! - return $mode if ( $mode & 0700 ) == ( 0400 | $attribs << 6 ); + if( ( $mode & 0700 ) == ( 0400 | $attribs << 6 )) { + $mode = sprintf("%d", $mode); + return sprintf("%d", $mode); + } $mode = 0444 | $attribs << 6 | $attribs << 3 | $attribs; - return $mode; + $mode = sprintf("%d", $mode); + return sprintf("%d", $mode); } sub unixFileAttributes { diff --git a/t/10_chmod.t b/t/10_chmod.t index 7ae647f..0495062 100644 --- a/t/10_chmod.t +++ b/t/10_chmod.t @@ -1,4 +1,4 @@ -#!/usr/bin/perl +#!/usr/bin/perl -T use strict; BEGIN {

Thu Oct 14 13:37:52 2010 The RT System itself - Status changed from 'new' to 'open'

Tue Nov 16 07:14:06 2010 http://www.openidfrance.fr/rosbif73 - Correspondence added

From:

John Wilcock

This taint issue crops up specifically with MailScanner 4.81.4 calling Archive::Zip 1.30 under perl 5.12.2 (all built from source on gentoo). Insecure dependency in chmod while running with -T switch at /usr/lib64/perl5/vendor_perl/5.12.2/Archive/Zip/Member.pm line 490 I can confirm that the proposed fix does indeed correct the issue.

Thu Jul 12 17:17:45 2012 http://viritrilbia.myopenid.com/ - Correspondence added

From:

Michael Shulman

This issue also arises using the TWiki BatchUploadPlugin, see http://twiki.org/cgi-bin/view/Plugins/BatchUploadPluginDev . The above patch fixes the issue there as well.