Insecure problem

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

Morning all!

I have a problem with my $ENV{PATH} when opening a file. here is what I did:

{
  $path = &launderFile($path);
  $ENV{PATH} .= ":$path";   # add the path we found to the environment
+ variable
  delete @ENV{qw(IFS CDPATH ENV BASH_ENV)};   # Make %ENV safer
  $command = &launderName($command);
  if (defined($output) && $output ne '') {
    if ($output eq $OUTHANDLER) {   # output is directed to a filehand
+ler and returned as an array
      if (! open(CMD, "$command 2>&1 |")) {
        warn "In performLocalCommand(): Can't open $command for readin
+g: $!";
        return;
      }
      &do_something();
    } else {
      ...
    }
  } else {   # ignores command output
    ...
  }
}
[download]

It all started when I concatenated $path to $ENV{PATH} instead of replacing $ENV{PATH}, I did that because the command I'm running needs not only it's path in order to run, it needs also the original path, that's why I can't override it.
Now I'm getting the Insecure $ENV{PATH}... error on the if (! open(CMD, "$command 2>&1 |")) { row.
anyone has an idea what's wrong?
Thanks

Hotshot

Comment on Insecure problem Download Code

Replies are listed 'Best First'.
Re: Insecure problem by lestrrat (Deacon) on Oct 23, 2002 at 10:59 UTC
Just a guess, but since $ENV{PATH} is already tainted, if you concatenate to $ENV{PATH}, it would still be tainted, right? so you probably need to extract out the paths that you explicitly want, and then reassign them with the new path that you want to add `## untested, just a guess my @path; foreach my $path ( split( /:/, $ENV{PATH} ) ) { if( is_allowed_path($path) ) { ## whatever that means... push @paths, $path; } } push @paths, "/some/other/path"; $ENV{PATH} = join( ':', @paths );` [download] Or maybe even, `delete $ENV{PATH}; $ENV{PATH} = "/path/to/tool;/some/other/path";` [download]	[reply] [d/l] [select]

Replies are listed 'Best First'.

Re: Insecure problem
by lestrrat (Deacon) on Oct 23, 2002 at 10:59 UTC

Just a guess, but since $ENV{PATH} is already tainted, if you concatenate to $ENV{PATH}, it would still be tainted, right?

so you probably need to extract out the paths that you explicitly want, and then reassign them with the new path that you want to add

    ## untested, just a guess
    my @path;
    foreach my $path ( split( /:/, $ENV{PATH} ) ) {
       if( is_allowed_path($path) ) { ## whatever that means...
           push @paths, $path;
       }
    }
    push @paths, "/some/other/path";

    $ENV{PATH} = join( ':', @paths );
[download]

Or maybe even,

     delete $ENV{PATH};
     $ENV{PATH} = "/path/to/tool;/some/other/path";
[download]

[reply]
[d/l]
[select]