Skip Menu |
 

This queue is for tickets about the Test-Compile CPAN distribution.

Report information
The Basics
Id: 55837
Status: resolved
Priority: 0/
Queue: Test-Compile

People
Owner: Nobody in particular
Requestors: ROBINS [...] cpan.org
Cc: mst [...] shadowcat.co.uk
AdminCc:

Bug Information
Severity: Normal
Broken in:
  • 0.08
  • 0.09
  • 0.10
  • 0.11
  • 0.12
  • 0.13
Fixed in: 0.17



Subject: pl_file_ok() method to execute perl doesn't work under taint mode
Download (untitled) / with headers
text/plain 905b
Instead of trying to explain the issue I'll let the IRC conversation I had with Matt S. Trout speak for itself. <robinsmidsrod> mst: I had a look in Test::Compile::pl_file_ok() - it uses my $out = `$^X -cw $file 2>&1`; <mst> oh wow <mst> that's completely broken <robinsmidsrod> mst: how else would you check if your script compiles without letting your current perl environment possibly clobber the test? <mst> that's -ish- fine but it'll fail to work under taint for e.g. <mst> you need to do something like <mst> system($^X, (map { "-I$_" } split ':', $ENV{PERL5LIB}), '-c', $file); <robinsmidsrod> maybe you should file a bug report on Test::Compile ;) <mst> maybe you should, since you're the one using it :) <robinsmidsrod> mst: well, I don't know how to quite articulate why that piece of code is broken <mst> make one of your scripts to #!perl -T <mst> that loads a module <mst> watch it blow up
I hope the attached patch may be of use. On Tue Mar 23 08:09:58 2010, ROBINS wrote: Show quoted text
> Instead of trying to explain the issue I'll let the IRC conversation I > had with Matt S. Trout speak for itself. > > <robinsmidsrod> mst: I had a look in Test::Compile::pl_file_ok() - it > uses my $out = `$^X -cw $file 2>&1`; > <mst> oh wow > <mst> that's completely broken > <robinsmidsrod> mst: how else would you check if your script compiles > without letting your current perl environment possibly clobber the
test? Show quoted text
> <mst> that's -ish- fine but it'll fail to work under taint for e.g. > <mst> you need to do something like > <mst> system($^X, (map { "-I$_" } split ':', $ENV{PERL5LIB}), '-c',
$file); Show quoted text
> <robinsmidsrod> maybe you should file a bug report on Test::Compile ;) > <mst> maybe you should, since you're the one using it :) > <robinsmidsrod> mst: well, I don't know how to quite articulate why
that Show quoted text
> piece of code is broken > <mst> make one of your scripts to #!perl -T > <mst> that loads a module > <mst> watch it blow up
Subject: a.txt
Download a.txt
text/plain 1018b
--- /dev/null +++ b/t/scripts/taint.pl @@ -0,0 +1,2 @@ +#!/usr/bin/perl -T +sleep 1; --- /dev/null +++ b/t/10_taint.t @@ -0,0 +1,7 @@ +#!perl -w +use strict; +use warnings; +use Test::More tests => 1; +use Test::Compile; +pl_file_ok('t/scripts/taint.pl', 'taint.pl compiles'); + --- a/lib/Test/Compile.pm +++ b/lib/Test/Compile.pm @@ -71,7 +71,8 @@ $Test->diag("$file does not exist"); return; } - my $out = `$^X -cw $file 2>&1`; + my $taint = _is_in_taint_mode($file); + my $out = `$^X -cw$taint $file 2>&1`; if ($?) { $Test->ok(0, 'Script does not compile'); $Test->diag($out); @@ -169,6 +170,19 @@ return 'script' if -e 'script'; return 'bin' if -e 'bin'; } + +sub _is_in_taint_mode { + my $file = shift; + open(FILE, $file) or die "could not open $file"; + my $shebang = <FILE>; + my $taint = undef; + if ($shebang =~ /^#![\/\w]+\s+\-w?(T)/) { + $taint = $1; + } + close FILE; + return $taint; +} + 1; __END__
Download (untitled) / with headers
text/plain 1.1k
Actually the $taint variable should be initialized with "" to avoid earnings. On Sat Nov 19 18:38:50 2011, SILASMONK wrote: Show quoted text
> I hope the attached patch may be of use. > > On Tue Mar 23 08:09:58 2010, ROBINS wrote:
> > Instead of trying to explain the issue I'll let the IRC conversation
I Show quoted text
> > had with Matt S. Trout speak for itself. > > > > <robinsmidsrod> mst: I had a look in Test::Compile::pl_file_ok() -
it Show quoted text
> > uses my $out = `$^X -cw $file 2>&1`; > > <mst> oh wow > > <mst> that's completely broken > > <robinsmidsrod> mst: how else would you check if your script
compiles Show quoted text
> > without letting your current perl environment possibly clobber the
> test?
> > <mst> that's -ish- fine but it'll fail to work under taint for e.g. > > <mst> you need to do something like > > <mst> system($^X, (map { "-I$_" } split ':', $ENV{PERL5LIB}), '-c',
> $file);
> > <robinsmidsrod> maybe you should file a bug report on Test::Compile
;) Show quoted text
> > <mst> maybe you should, since you're the one using it :) > > <robinsmidsrod> mst: well, I don't know how to quite articulate why
> that
> > piece of code is broken > > <mst> make one of your scripts to #!perl -T > > <mst> that loads a module > > <mst> watch it blow up
> >
Download (untitled) / with headers
text/plain 158b
I'm really not very experienced in the taint mode of Perl, so I added Matt Trout (mst) to the thread. Maybe he can tell you if the patch makes sense or not.
Download (untitled) / with headers
text/plain 216b
On Mon Nov 28 04:44:16 2011, ROBINS wrote: Show quoted text
> I'm really not very experienced in the taint mode of Perl, so I added > Matt Trout (mst) to the thread. > > Maybe he can tell you if the patch makes sense or not.
Evan,
Subject: taint.patch
Download taint.patch
text/x-diff 1.8k
Author: Nicholas Bamber <nicholas@periapt.co.uk> Subject: taint mode not respected If the -T argument is passed on the command line to the perl executable, it will turn on "taint" treating all input as suspect until checked, and dieing if the scripts attempts to output tainted data. Using taint mode is considered good practice for sensitive programs that could possibly be run by untrusted users. If the -T argument is used in the shebang line of the script, then it needs to be passed when the script is invoked - otherwise the script will fail to compile. Thus it is quite important that this module pass the -T flag when required. We also provide a test script to verify the extra functionality. There is also a -t argument which warns rather than dies. Last-Update: 2012-02-18 Bug-Debian: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=649301 Bug: https://rt.cpan.org/Public/Bug/Display.html?id=55837 --- /dev/null +++ b/t/10_taint.t @@ -0,0 +1,7 @@ +#!perl -w +use strict; +use warnings; +use Test::More tests => 1; +use Test::Compile; +pl_file_ok('t/scripts/taint.pl', 'taint.pl compiles'); + --- a/lib/Test/Compile.pm +++ b/lib/Test/Compile.pm @@ -138,7 +138,8 @@ return ($@ ? 0 : 1); } else { my @perl5lib = split(':', ($ENV{PERL5LIB}||"")); - system($^X, (map { "-I$_" } @perl5lib), '-c', $file); + my $taint = _is_in_taint_mode($file); + system($^X, (map { "-I$_" } @perl5lib), "-c$taint", $file); return ($? ? 0 : 1); } } @@ -177,6 +178,19 @@ return 'script' if -e 'script'; return 'bin' if -e 'bin'; } + +sub _is_in_taint_mode { + my $file = shift; + open(FILE, $file) or die "could not open $file"; + my $shebang = <FILE>; + my $taint = ""; + if ($shebang =~ /^#![\/\w]+\s+\-w?([tT])/) { + $taint = $1; + } + close FILE; + return $taint; +} + 1; __END__
Download (untitled) / with headers
text/plain 216b
On Mon Nov 28 04:44:16 2011, ROBINS wrote: Show quoted text
> I'm really not very experienced in the taint mode of Perl, so I added > Matt Trout (mst) to the thread. > > Maybe he can tell you if the patch makes sense or not.
Evan,
Download (untitled) / with headers
text/plain 216b
On Mon Nov 28 04:44:16 2011, ROBINS wrote: Show quoted text
> I'm really not very experienced in the taint mode of Perl, so I added > Matt Trout (mst) to the thread. > > Maybe he can tell you if the patch makes sense or not.
Evan,
Download (untitled) / with headers
text/plain 216b
On Mon Nov 28 04:44:16 2011, ROBINS wrote: Show quoted text
> I'm really not very experienced in the taint mode of Perl, so I added > Matt Trout (mst) to the thread. > > Maybe he can tell you if the patch makes sense or not.
Evan,
Download (untitled) / with headers
text/plain 216b
On Mon Nov 28 04:44:16 2011, ROBINS wrote: Show quoted text
> I'm really not very experienced in the taint mode of Perl, so I added > Matt Trout (mst) to the thread. > > Maybe he can tell you if the patch makes sense or not.
Evan,
On Sun Feb 19 07:50:26 2012, SILASMONK wrote: Show quoted text
> On Mon Nov 28 04:44:16 2011, ROBINS wrote:
> > I'm really not very experienced in the taint mode of Perl, so I
added Show quoted text
> > Matt Trout (mst) to the thread. > > > > Maybe he can tell you if the patch makes sense or not.
> > Evan,
Evan, I have refreshed the patch. I was rather annoyed that the the code had changed substantially without actually addressing the taint or lib issues. I attempted to contact you as per this email: http://lists.debian.org/debian-perl/2012/02/msg00064.html . I was even more annoyed when this email bounced, especially as "egiles@cpan.org" is listed as a contact in the Test::Compile. I then attempted to contact you via github which failed. I did however realize at this point that you are finally looking at this issue. I should however point out that the _is_in_taint_mode function already returns whatever needs to go in the command line arguments and does not require any further modification. In fact there is also a lesser used "-t" mode (only warns) which your draft version would mess up.
Download (untitled) / with headers
text/plain 1.1k
Patch included On Sun Feb 19 08:02:50 2012, SILASMONK wrote: Show quoted text
> On Sun Feb 19 07:50:26 2012, SILASMONK wrote:
> > On Mon Nov 28 04:44:16 2011, ROBINS wrote:
> > > I'm really not very experienced in the taint mode of Perl, so I
> added
> > > Matt Trout (mst) to the thread. > > > > > > Maybe he can tell you if the patch makes sense or not.
> > > > Evan,
> > Evan, > I have refreshed the patch. I was rather annoyed that the the code > had changed substantially without actually addressing the taint or lib > issues. I attempted to contact you as per this email: > http://lists.debian.org/debian-perl/2012/02/msg00064.html . I was even > more annoyed when this email bounced, especially as "egiles@cpan.org"
is Show quoted text
> listed as a contact in the Test::Compile. I then attempted to contact > you via github which failed. I did however realize at this point that > you are finally looking at this issue. I should however point out that > the _is_in_taint_mode function already returns whatever needs to go in > the command line arguments and does not require any further > modification. In fact there is also a lesser used "-t" mode (only
warns) Show quoted text
> which your draft version would mess up.
Subject: taint.patch
Download taint.patch
text/x-diff 1.8k
Author: Nicholas Bamber <nicholas@periapt.co.uk> Subject: taint mode not respected If the -T argument is passed on the command line to the perl executable, it will turn on "taint" treating all input as suspect until checked, and dieing if the scripts attempts to output tainted data. Using taint mode is considered good practice for sensitive programs that could possibly be run by untrusted users. If the -T argument is used in the shebang line of the script, then it needs to be passed when the script is invoked - otherwise the script will fail to compile. Thus it is quite important that this module pass the -T flag when required. We also provide a test script to verify the extra functionality. There is also a -t argument which warns rather than dies. Last-Update: 2012-02-18 Bug-Debian: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=649301 Bug: https://rt.cpan.org/Public/Bug/Display.html?id=55837 --- /dev/null +++ b/t/10_taint.t @@ -0,0 +1,7 @@ +#!perl -w +use strict; +use warnings; +use Test::More tests => 1; +use Test::Compile; +pl_file_ok('t/scripts/taint.pl', 'taint.pl compiles'); + --- a/lib/Test/Compile.pm +++ b/lib/Test/Compile.pm @@ -138,7 +138,8 @@ return ($@ ? 0 : 1); } else { my @perl5lib = split(':', ($ENV{PERL5LIB}||"")); - system($^X, (map { "-I$_" } @perl5lib), '-c', $file); + my $taint = _is_in_taint_mode($file); + system($^X, (map { "-I$_" } @perl5lib), "-c$taint", $file); return ($? ? 0 : 1); } } @@ -177,6 +178,19 @@ return 'script' if -e 'script'; return 'bin' if -e 'bin'; } + +sub _is_in_taint_mode { + my $file = shift; + open(FILE, $file) or die "could not open $file"; + my $shebang = <FILE>; + my $taint = ""; + if ($shebang =~ /^#![\/\w]+\s+\-w?([tT])/) { + $taint = $1; + } + close FILE; + return $taint; +} + 1; __END__
Download (untitled) / with headers
text/plain 1.1k
On Sun Feb 19 08:03:49 2012, SILASMONK wrote: Show quoted text
> Patch included > >
> > Evan, > > I have refreshed the patch. I was rather annoyed that the the
code Show quoted text
> > had changed substantially without actually addressing the taint or
lib Show quoted text
> > issues. I attempted to contact you as per this email: > > http://lists.debian.org/debian-perl/2012/02/msg00064.html . I was
even Show quoted text
> > more annoyed when this email bounced, especially as
"egiles@cpan.org" Show quoted text
> is
> > listed as a contact in the Test::Compile. I then attempted to
contact Show quoted text
> > you via github which failed. I did however realize at this point
that Show quoted text
> > you are finally looking at this issue. I should however point out
that Show quoted text
> > the _is_in_taint_mode function already returns whatever needs to go
in Show quoted text
> > the command line arguments and does not require any further > > modification. In fact there is also a lesser used "-t" mode (only
> warns)
> > which your draft version would mess up.
>
Wow, you raise quite a few points there, and to be honest, they are mostly justified. I apologise for making you task more difficult than necessary. Please leave it with me for a short while, and I will endeavour to resolve those issues as soon as possible.
Download (untitled) / with headers
text/plain 226b
I have now released v0.17 of Test::Compile to CPAN. This version includes the supplied patch, almost unchanged, and I am fairly certain that it resolves the your issue. I hope this is an acceptable solution to all involved.


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.