in reply to Re^4: Insecure dependency in system under -T, with list form invocation
in thread Insecure dependency in system under -T, with list form invocation

You need to understand what this code does: 

#!/usr/local/bin/perl -T
use warnings;
use strict;
use Scalar::Util qw( tainted );

print "ARGV: " . ( tainted( @ARGV ) + 0 ) . "\n";
print "ARGV[0]: " . ( tainted( $ARGV[0] ) + 0 ) . "\n" if defined $ARG
+V[0];

[download]
When run as ./program foo you get this: 
ARGV: 0
ARGV[0]: 1

[download]

Taintedness is a property of a scalar. An array is not tainted, but the scalar elements of it are. You need to untaint the elements of your arrays that are being passed as arguments.

Replies are listed 'Best First'.
Re^6: Insecure dependency in system under -T, with list form invocation
by cramdorgi (Acolyte) on Sep 12, 2008 at 08:24 UTC
    Thanks,
    I got this indeed by myself in the meanwhile.
    I would object that the tainted function could easily avoid giving misleading information to naive users...

    But no problem: it is useful as it is.
    Now wouldn't something like the following be of value? 

    sub untaint($) {
  my $tainted = shift;
  my @untaintedbits;
  foreach (split //, $tainted) {
    if (m%([-\@\w.])%) {
      push @untaintedbits, $1;
    }
  }
  return join '', @untaintedbits;
}
sub untaintunixpath($) {
  my $tainted = shift;
  my @dirs = split '/', $tainted;
  map { $_ = untaint($_) } @dirs;
  return join '/', @dirs;
}
sub untaintstring($) {
  my $tainted = shift;
  my @words = split /\s+/, $tainted;
  map { $_ = untaint($_) } @words;
  return join ' ', @words;
}
my $res = GetOptions("help" => \$help, "unlock" => \$unlock, "vob=s" =
+> \$vob,
             "nusers=s" => \@nusers, "lbtype=s" => \@lbtype);
usage if $help or !($res and $vob and @lbtype) or ($unlock and @nusers
+);
@lbtype = split(/,/, join(',', @lbtype));
map { $_ = untaint($_) } @lbtype;
$vob = untaintunixpath($vob);
$vob = $ct->argv(qw(des -s), "vob:$vob")->qx;
die "Couldn't find the vob $vob\n" unless $vob;
$vob = untaintunixpath($vob);
my $pwnam = (getpwuid($<))[6];
$pwnam =~ s/^ *(.*[^ ]) *$/$1/;
$pwnam = untaintstring($pwnam);

    [download]
    etc...

    Marc

[reply]
[d/l]
      Wrapping the untainting into a sub for things that should be untainted the same way does make sense.

      Your specific regex for untaining Unix paths works for common cases, but not for everything that's a valid path on most file systems. If you really want to make sure only that regex matches and ignore the more general case, that's fine but then the name of the sub is a bit misleading. Perhaps something like "untaintsimpleunixpath" or "untaintcommonunixpath". Alternately, having some good documentation that explains the limited range of paths it will allow could help clear up any confusion. Always assume you're writing code for another programmer as much as for the computer. If you're that programmer six months from now, you just might thank yourself.

      On a stylistic note, and this is a personal preference but one that's pretty common, don't be afraid to use underscores between words in subroutine names. I find "untaint_unix_path" much more readable than "untaintunixpath". Some others would rewrite that as "untaintUnixPath", which I find clearer than the all-lowercase version but not as clear and the underscored version.

[reply]

In Section Seekers of Perl Wisdom