Re^6: Insecure dependency in system under -T, with list form invocation

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

Comment on Re^6: Insecure dependency in system under -T, with list form invocation Download Code

Replies are listed 'Best First'.
Re^7: Insecure dependency in system under -T, with list form invocation by mr_mischief (Monsignor) on Sep 12, 2008 at 15:01 UTC
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]

Replies are listed 'Best First'.

Re^7: Insecure dependency in system under -T, with list form invocation
by mr_mischief (Monsignor) on Sep 12, 2008 at 15:01 UTC

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]