Skip Menu |

This queue is for tickets about the File-Find-Rule CPAN distribution.

Report information
The Basics
Id: 20418
Status: resolved
Priority: 0/
Queue: File-Find-Rule

People
Owner: Nobody in particular
Requestors: zev [...] cpan.org
Cc:
AdminCc:

Bug Information
Severity: (no value)
Broken in: (no value)
Fixed in: (no value)



Subject: File::Find::Rule is not taint-safe
To make File::Find::Rule taint-safe, it would require the ability to set the taint options and pass them to the find call. I'd be happy to write the code, but I don't know the correct way to pass options to ::Rule.
This patch makes it work when in taint mode, HTH :)
--- /usr/lib/perl5/site_perl/5.8.7/File/Find/Rule.pm.orig 2006-11-13 22:36:22.000000000 -0600 +++ /usr/lib/perl5/site_perl/5.8.7/File/Find/Rule.pm 2006-11-13 22:36:50.000000000 -0600 @@ -578,6 +578,9 @@ my $sub = eval "$code" or die "compile error '$code' $@"; my $cwd = getcwd; + $cwd =~ /(.*)/; + $cwd = $1; + for my $path (@_) { # $topdir is used for relative and maxdepth $topdir = $path; @@ -587,6 +590,7 @@ unless $topdir eq '/'; $self->_call_find( { %{ $self->{extras} }, wanted => $sub }, $path ); } + chdir $cwd; return @found;
From: GWOLF [...] cpan.org
This bug was reported as well in the Debian BTS (http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=406205) - I'm pasting here the user's report, as well as his proposed patch: On etch, the taint mode won't work: $ perl -MFile::Find::Rule -Tle '$rule=File::Find::Rule->new->extras({ untaint => 1 })->start($ARGV[0]); while ($f = $rule->match ) { print $f; }' . Insecure dependency in chdir while running with -T switch at /usr/share/perl5/File/Find/Rule.pm line 591. This _will_ render alls scripts, program and applications using taint mode and this module unusable. The bug is listed at CPAN for about 2 months now: http://rt.cpan.org/Public/Bug/Display.html?id=20418 However, the untainting in that patch is just a slob job. Mine does real untainting. Patch (taken from working version 0.28 from sarge): --- Rule.pm.030 2007-01-09 16:10:53.000000000 +0100 +++ Rule.pm.030.fixed 2007-01-09 16:10:53.000000000 +0100 @@ -575,6 +575,12 @@ my $sub = eval "$code" or die "compile error '$code' $@"; my $cwd = getcwd; + # Untaint it + if ( $cwd =~ qr|^([-+@\w./]+)$| ) { + $cwd = $1; + } else { + die "Couldn't untaint \$cwd: [$cwd]"; + } for my $path (@_) { # $topdir is used for relative and maxdepth $topdir = $path; -- System Information: Debian Release: 4.0 APT prefers testing APT policy: (700, 'testing'), (500, 'unstable') Architecture: i386 (i686) Shell: /bin/sh linked to /bin/bash Kernel: Linux 2.6.18-3-686 Locale: LANG=en_GB, LC_CTYPE=en_GB (charmap=ISO-8859-15) Versions of packages libfile-find-rule-perl depends on: ii libnumber-compare-perl 0.01-4 Perform numeric comparisons in Per ii libtext-glob-perl 0.07-1 Match globbing patterns against te ii perl 5.8.8-7 Larry Wall's Practical Extraction ii perl-modules 5.8.8-7 Core Perl modules libfile-find-rule-perl recommends no packages. -- no debconf information
And I'm also copying my reply to the user's bug report in Debian, of course: tag 406205 + upstream severity 406205 minor thanks Wolfgang Schemmel dijo [Tue, Jan 09, 2007 at 04:17:38PM +0100]: Show quoted text
> On etch, the taint mode won't work: > > $ perl -MFile::Find::Rule -Tle '$rule=File::Find::Rule->new->extras({
untaint => 1 })->start($ARGV[0]); while ($f = $rule->match ) { print $f; }' . Show quoted text
> Insecure dependency in chdir while running with -T switch at
/usr/share/perl5/File/Find/Rule.pm line 591. Show quoted text
> > This _will_ render alls scripts, program and applications > using taint mode and this module unusable. > > The bug is listed at CPAN for about 2 months now: > http://rt.cpan.org/Public/Bug/Display.html?id=20418 > However, the untainting in that patch is just a slob job. > Mine does real untainting.
Umh... I'm not sure I like this. I think the taint mode _is_ working correctly here - You are getting information from outside your program's direct control (the result of getcwd), and that perfectly qualifies as tainting. And, although you include a check against a regular expression, AFAICT it's a pretty arbitrary one: Show quoted text
> my $cwd = getcwd; > + # Untaint it > + if ( $cwd =~ qr|^([-+@\w./]+)$| ) { > + $cwd = $1; > + } else { > + die "Couldn't untaint \$cwd: [$cwd]"; > + }
Many users (although it's against the Unix culture) use spaces inside files and directories (i.e. many of my users have their "My Documents" Windows directory backed up in my server). Or filenames with all kinds of diacritical marks on them, which would fail your test. But still, you are not untainting the information - you are just giving a hopefully correct pattern, still subject to containing wrong information. If anything, I would recommend changing the module's behaviour in a way that the user should specify that he _knows_ some taintedness will enter this way (although very probably benign, system-generated taintedness), i.e., invoking this way (for the documentation's first example): my @subdirs = File::Find::Rule->directory(untaint=>1)->in( $directory ); PS- I'm following up this report to the upstream bug you mentioned, as it belongs to upstream development and not in Debian. Greetings, -- Gunnar Wolf - gwolf@gwolf.org - (+52-55)5623-0154 / 1451-2244 PGP key 1024D/8BB527AF 2001-10-23 Fingerprint: 0C79 D2D1 2C4E 9CE4 5973 F800 D80E F35A 8BB5 27AF
The next release, currently under testing, will be taint-safe by virtue of being based around File::Next rather than File::Find. -- Richard Clamp <richardc@unixbeard.net>
On Tue Jun 19 12:19:24 2007, RCLAMP wrote: Show quoted text
> The next release, currently under testing, will be taint-safe by
virtue Show quoted text
> of being based around File::Next rather than File::Find.
OK, it's 2009 - is there an update on the promised fix? If the 'next release' isn't finished, it occurs to me that extras() supplies the necessary information to File::Find; all File::Find:Rule needs to do is look inside the options hash and obey untaint & untaint_pattern (defaulting the latter as FileFind does to qr|^([- +@\w./]+)$|. E.G. replace the chdir at ~ line 590 with: if( $self->{extras}{untaint} ) { $cwd =~ ( $self->{extras}{untaint_pattern} || qr|^([-+@\w./]+) $| ); $cwd = $1; } chdir $cwd; This should solve 90% of the problems - for extra credit, one could do untaint_skip as well.
0.31 just released to CPAN makes it as taint-unsafe as the underlying File::Find module. -- Richard Clamp <richardc@unixbeard.net>