Unlink under taint mode

Andre_br has asked for the wisdom of the Perl Monks concerning the following question:

My dear fellow programmers

I am in need of thy help!

I have just activated the -T switch in all my scripts and everything went smoothly untill I run into the "unlink" command. He seems not to accept my untainting regexp, complaining "Insecure dependency in unlink while running with -T switch...". Here goes my code:


my $subcookie = untaint ( substr($cookie,0,6) ); # this is a substring
+ of the SID, that I use as a name for a dinamic directory for the use
+r. Also, check my untaint() function below
      
opendir (DIR, "../users/$subcookie/"); # I was using glob, but couldn´
+t make -T accept it, so I changed to readdir
         
my @files = readdir DIR;
      
foreach my $file (@files) { 
      
   my $checked_file;
      
   if ( $file =~ /(\w+)\.(\w{3,4})/ ) { $checked_file = "$1.$2"; } # a
+voiding the first "." and ".." thar readdir returns            
                 
   if ( defined $checked_file ) { unlink "../users/$subcookie/$checked
+_file"; }
               
         
}
         
close DIR;

... # and the code goes on
[download]

Here´s the untaint() function, wich is 'require'd from another .cgi:


sub untaint {

      my $string = shift;
   
      my $clean_string;
     
      if ( $string =~ /([\w\-\_]+)/ ) { $clean_string = $1; } 
   
      else { die "ilegal character: $!"; }
   
      return $clean_string;
      
}
[download]

Anyone knows what might be? Is there a less demanding function I can use to wipe out all the files in this directory?

Thanks a lot, my friends!

André

Comment on Unlink under taint mode Select or Download Code

Replies are listed 'Best First'.
Re: Unlink under taint mode by tlm (Prior) on Apr 10, 2005 at 01:48 UTC
I can't immediately see the problem with your code, but what I would do is to use the `tainted` function from Scalar::Util to pinpoint the exactly which variable is giving you trouble. BTW, to avoid `.` and `..`, all you need to do is `my @files = map untaint( $_ ), grep !/^\.\.?$/, readdir DIR;` [download] or better yet `my @files = map untaint( $_ ), grep -f "$dir/$_", readdir DIR;` [download] Also, the regexp in your `untaint` function could be written more economically as `/([\w-]+)/` [download] `\w` already implies `_`, and there's no need to escape `-` (nor `_` BTW) inside the `[ ]` as long as it is the first or the last character in the group (e.g. `[-AZ]` matches `-`, `A`, and `Z`.). the lowliest monk	[reply] [d/l] [select]
Follow up by Andre_br (Pilgrim) on Apr 10, 2005 at 02:12 UTC
Hey Postulant, Thanks for the tips on the regexp. But, as Perl doesn´t mind about the regexp I use, for what concerns -T, I still wonder what´s wrong. The "." and ".." are properly skipped, because I only run the unlink if the checked file variable is defined. I was wondering: may the problem be the path?? In the command, unlink "../users/$subcookie/$checked_file", both $subookie and $checked_file have been untainted. The only one thing this command still relies is the path! How can I clean up the path?? (It´s strange that this path wasn´t problem before when I even opened files for writting and stuff; maybe if unlink is more demanding about security, don´t know...) Any hints? Thanks a lot André	[reply]
Re: Follow up by tlm (Prior) on Apr 10, 2005 at 02:31 UTC
The important part of my earlier reply was the bit about `Scalar::Util::tainted` (which I would just repeat now); the stuff about the regexps was just BTW. the lowliest monk	[reply] [d/l]
Re: Unlink under taint mode by Zaxo (Archbishop) on Apr 10, 2005 at 09:52 UTC
By using a relative path, you have an implicit dependence on $ENV{PWD}, which is tainted along with the rest of the environment. Give an absolute path if possible (by untainting $ENV{DOCUMENT_ROOT} if necessary). After Compline, Zaxo	[reply]
Re^2: Unlink under taint mode by tlm (Prior) on Apr 10, 2005 at 15:16 UTC
I don't think that's it, or at least, that's not the way it works on one Linux/Apache configuration. For one thing, `$ENV{ PWD }` is not even in the environment of CGI scripts (when run under Apache) in this configuration; here are the keys of `%ENV`, printed from within one such script: `AUTH_TYPE HTTP_KEEP_ALIVE SCRIPT_FILENAME DOCUMENT_ROOT HTTP_USER_AGENT SCRIPT_NAME GATEWAY_INTERFACE PATH SERVER_ADDR HTTP_ACCEPT PERL5LIB SERVER_ADMIN HTTP_ACCEPT_CHARSET QUERY_STRING SERVER_NAME HTTP_ACCEPT_ENCODING REMOTE_ADDR SERVER_PORT HTTP_ACCEPT_LANGUAGE REMOTE_PORT SERVER_PROTOCOL HTTP_CONNECTION REMOTE_USER SERVER_SIGNATURE HTTP_COOKIE REQUEST_METHOD SERVER_SOFTWARE HTTP_HOST REQUEST_URI` [download] Furthermore, if I run the CGI script listed below on my server, untainting the input filename (by passing a non-zero `untaint` param to the script) is sufficient to appease `-T`, even though I am creating and unlinking a file in a relative directory (`../TRASH`); if the filename is not untainted, however, `-T` kills the script at the unlinking step. Read more... (1061 Bytes) the lowliest monk	[reply] [d/l] [select]
Re: Unlink under taint mode by tlm (Prior) on Apr 10, 2005 at 16:26 UTC
I had another thought: does your code `use locale`? (I noticed that your location is Brazil.) Under `use locale`, `perl -T` won't trust `/\w/` unless you specifically tell it so. For example, in your `untaint` you would use: `{ no locale; if ( $string =~ /([\w\-\_]+)/ ) { $clean_string = $1; } else { die "ilegal character: $!"; } }` [download] See Laundering and Detecting Tainted Data in `perlsec`, and the SECURITY section of `perllocale`. the lowliest monk	[reply] [d/l] [select]
Right on the point! by Andre_br (Pilgrim) on Apr 12, 2005 at 00:37 UTC
Hey Postulant, That´s exactly the problem! Thanks a lot! I do use a specific locale. I had already managed to clean the $string out, with a pattern that didn´t have the \w and didn´t know why I succeeded. That´s it. By the way, I´ve found a simpler way to test if a variable is tainted, without having to use any extra module: `if ( is_tainted($input) ) { die "tainted"; } else { die "not tainted"; + } sub is_tainted { return ! eval { join('',@_), kill 0; 1; }; }` [download] Thank you all! Take Care André	[reply] [d/l]