Skip Menu |

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

Report information
The Basics
Id: 26920
Status: resolved
Priority: 0/
Queue: File-Slurp

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

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



Subject: read_file() in list context untaints file content
Hello, I've just discovered that read_file() in list context untaints the content of the file it's returning: maddingue@fangorn:~ $ echo 0wn3d >random_file maddingue@fangorn:~ $ perl -wTMDevel::Peek -MFile::Slurp -e'$data=read_file(shift); Dump($data);' random_file SV = PVMG(0x80856f8) at 0x804ccd4 REFCNT = 1 FLAGS = (GMG,SMG,pPOK) IV = 0 NV = 0 PV = 0x8067738 "0wn3d\n"\0 CUR = 6 LEN = 7 MAGIC = 0x81282e0 MG_VIRTUAL = &PL_vtbl_taint MG_TYPE = PERL_MAGIC_taint(t) MG_LEN = 1 maddingue@fangorn:~ $ perl -wTMDevel::Peek -MFile::Slurp -e'$data=join"",read_file(shift); Dump($data);' random_file SV = PV(0x804d054) at 0x804ccd4 REFCNT = 1 FLAGS = (POK,pPOK) PV = 0x811ec78 "0wn3d\n"\0 CUR = 6 LEN = 7 The reason is that the return in line 205 uses m//. Confirmed with various versions of Perl 5.6 and 5.8. Regards, -- Close the world, txEn eht nepO.
From: SAPER [...] cpan.org
Here is a patch that adds a test script to demonstrate this bug. -- Close the world, txEn eht nepO.
diff -ruN File-Slurp-9999.12-orig/t/taint.t File-Slurp-9999.12/t/taint.t --- File-Slurp-9999.12-orig/t/taint.t 1970-01-01 01:00:00.000000000 +0100 +++ File-Slurp-9999.12/t/taint.t 2007-06-08 16:15:42.852350224 +0200 @@ -0,0 +1,30 @@ +#!perl -T +use strict; +use Test::More; +use File::Slurp; +use Scalar::Util qw(tainted); + +plan tests => 4; + +my $path = "data.txt"; +my $data = "random junk\n"; + +SKIP: { + # write something to that file + open(FILE, ">$path") or skip 4, "can't write to '$path': $!"; + print FILE $data; + close(FILE); + + # read the file using File::Slurp in scalar context + my $content = eval { read_file($path) }; + is( $@, '', "read_file() in scalar context" ); + ok( tainted($content), " => returned content should be tainted" ); + + # read the file using File::Slurp in list context + my @content = eval { read_file($path) }; + is( $@, '', "read_file() in list context" ); + ok( tainted($content[0]), " => returned content should be tainted" ); +} + +unlink $path; +
From: SAPER [...] cpan.org
Here is finaly a patch that fixes the issue. I'd also suggest to replace the ok( eq_array(..), .. ) in tests with is_deeply(). Doing this helped me to understand why some of the tests were failing while trying to fix File::Slurp. -- Close the world, txEn eht nepO.
diff -ru File-Slurp-9999.12-orig/lib/File/Slurp.pm File-Slurp-9999.12/lib/File/Slurp.pm --- File-Slurp-9999.12-orig/lib/File/Slurp.pm 2006-02-17 07:13:51.000000000 +0100 +++ File-Slurp-9999.12/lib/File/Slurp.pm 2007-06-12 10:51:25.953969291 +0200 @@ -6,6 +6,7 @@ use POSIX qw( :fcntl_h ) ; use Fcntl qw( :DEFAULT ) ; use Symbol ; +BEGIN { eval 'use UNIVERSAL::isa' } my $is_win32 = $^O =~ /win32/i ; @@ -85,7 +86,7 @@ # check if we are reading from a handle (glob ref or IO:: object) - if ( ref $file_name ) { + if ( ref $file_name && (UNIVERSAL::isa($file_name, 'GLOB') || UNIVERSAL::isa($file_name, 'IO')) ) { # slurping a handle so use it and don't open anything. # set the block size so we know it is a handle and read that amount @@ -195,15 +196,34 @@ # this split doesn't work since it tries to use variable length lookbehind # the m// line works. # return [ split( m|(?<=$sep)|, ${$buf_ref} ) ] if $args{'array_ref'} ; - return [ length(${$buf_ref}) ? ${$buf_ref} =~ /(.*?$sep|.+)/sg : () ] - if $args{'array_ref'} ; +# return [ length(${$buf_ref}) ? ${$buf_ref} =~ /(.*?$sep|.+)/sg : () ] +# if $args{'array_ref'} ; # caller wants a list of lines (normal list context) # same problem with this split as before. # return split( m|(?<=$sep)|, ${$buf_ref} ) if wantarray ; - return length(${$buf_ref}) ? ${$buf_ref} =~ /(.*?$sep|.+)/sg : () - if wantarray ; +# return length(${$buf_ref}) ? ${$buf_ref} =~ /(.*?$sep|.+)/sg : () +# if wantarray ; + if (wantarray || $args{'array_ref'}) { + # split the text on the end-of-line separator + my @lines = split /($sep)/, ${$buf_ref}; + + # reconstruct the full lines by merging items by pairs + for my $k (0..int($#lines/2)) { + my $i = $k * 2; + $lines[$k] = (defined $lines[$i] ? $lines[$i] : '') + . (defined $lines[$i+1] ? $lines[$i+1] : ''); + } + + # remove the rest of the items + splice(@lines, int($#lines/2)+1); + pop @lines unless $lines[-1]; + + # return as array of arrayref + return $args{'array_ref'} ? \@lines : @lines + } + # caller wants a scalar ref to the slurped text
fixed