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

in reply to Insecure dependency in system under -T, with list form invocation

As many people have pointed out already, one of your arguments to system is tainted. Unfortunately, Perl doesn't tell you which one. You could walk though your code to try and figure it out, but since we're programming in Perl, laziness is a virtue, and there are modules to check this for you automatically.

If you have a recent version of IPC::System::Simple then if called it tainted arguments it will tell you which ones are tainted. That means your code changes to:

use IPC::System::Simple qw(system);

system('/usr/bin/ssh', '-l', $owner, $host, $binct, @op, @lbtype);
[download]

IPC::System::Simple will also throw a detailed exception if your ssh command doesn't run, is killed by a signal, or returns a non-zero exit value (although you can change that if you wish, see the docs).

If you're using autodie and have IPC::System::Simple installed, then you can do the same thing with lexical scope (ie, just for that block):

{
    use autodie qw(system);
    system('/usr/bin/ssh', '-l', $owner, $host, $binct, @op, @lbtype);
}
[download]

IPC::System::Simple is pure Perl with no dependencies and works on 5.6.0 and above. autodie is pure Perl, has IPC::System::Simple as an optional dependency, and works on 5.8.0 or above.

Disclaimer: I wrote all the modules mentioned in this post, so I obviously think they're great. ;)

Best regards,

Paul Fenwick
Perl Training Australia

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

Replies are listed 'Best First'.
Re^2: Insecure dependency in system under -T, with list form invocation by cramdorgi (Acolyte) on Sep 11, 2008 at 13:12 UTC
Hi, Warm thanks to all who replied... I was not expecting to reach ClearCase users, so that I showed the whole code merely in an attempt to show that I had a real context, and a good reason for not providing a reproducible case. Under the debugger, and using `Scalar::Util::tainted`, I could indeed find that the culprit is `$owner`... (despite my regexp processing, and some further more drastic, which failed so far). I did install and start to use IPC::System::Simple and autodie, but so far without success. I add below the updated code, and the transcript. Take this as an intermediate report until I get out of my mess (I of course take any help you might want to give me, but I ought to be able to find my way alone now--I still hope so anyway) I use `$me` in the usage function. The `Name "main::LOG" used only once` error goes away if I remove `use autodie` Actually, `use IPC::System::Simple` doesn't seem to make any differnce either... Now, I untainted everything and still get the error... See the diffs and the last bit of transcript below... Added back `use IPC::System::Simple;` and `use autodie qw(:system);` (i.e. now, explicitly with system), and got something different (and useful). See below, Sep 11 17:43 update. Breakthrough: this worked! The `tainted` function of Scalar::Util doesn't work well with strings... Thanks again, especially to Paul for his useful modules! Marc #!/vobs/cello/cade_struct/bin/perl -w use strict; use Sys::Hostname; use File::Basename; use Getopt::Long; use ClearCase::Argv; use IPC::System::Simple; use autodie; use vars qw($help $unlock $vob @op @nusers @lbtype); my $me = basename($0); my $host = hostname; my $ssh = '/usr/bin/ssh'; my $binct = '/opt/rational/clearcase/bin/cleartool'; my $account = getlogin \|\| getpwuid($<) or die "Couldn't get the uid: $ +!\n"; my $eaccount = getpwuid($>) or die "Couldn't get the euid: $!\n"; my $log = "/home/$eaccount/RANOS_autobuild/builds/logs/lbunlock.log"; $ENV{PATH} = ''; ClearCase::Argv->ipc(1); my $ct = ClearCase::Argv->new({autochomp=>1}); sub usage() { print "Usage: ${me} [[--unlock [\| --nusers accounts]] --vob <vob>\n" . " --lbtype <lbtypes> \| [--help]]\n" . " By default, lock; use --unlock explicitely.\n" . " Only one vob is accepted, and it is mandatory.\n" . " Multiple label types are possible, either with separate +options" . "\n or as one comma separated list.\n" . " All the types must exist in the vob.\n"; exit 1; } 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)); my $vob = $ct->argv(qw(des -s), "vob:$vob")->qx; die "Couldn't find the vob $vob\n" unless $vob; my $pwnam = (getpwuid($<))[6]; $pwnam =~ s/^ (.[^ ]) $/$1/; if ($unlock) { my @t = localtime; my $t = sprintf"%4d%02d%02d.%02d:%02d:%02d", (1900+$t[5]),1+$t[4],$t[3],$t[2],$t[1],$t[0]; open LOG, ">>", "$log" or die "Failed to open the $log log: $!\n"; print LOG "$t $account $vob @lbtype\n"; close LOG; @op = ('unlock'); } else { @op = ('lock', '-c', "'Actual lock author: $account $$pwnam$'"); push(@op, '-nusers', join(',', @nusers)) if @nusers; } my ($owner) = grep s%^./(.*)$%$1%, $ct->argv(qw(des -fmt "%[owner]p"), "vob:$vob")->qx; map { $_ = "lbtype:$_\@$vob" } @lbtype; foreach my $t (@lbtype) { $ct->argv(qw(des -s), $t)->stdout(0)->system and die "Label type $t not found in $vob\n"; } $< = $>; $owner =~ s/[^-\@\w.]//g; system($ssh, '-l', $owner, $host, $binct, @op, @lbtype); map{print"$_\n"}($ssh, '-l', $owner, $host, $binct, @op, @lbtype); [download] `$ ~eeivob05/bin/locklbtype -vob /vobs/atcctest -l MG -u Name "main::LOG" used only once: possible typo at /dev/fd/4 line 45. Insecure dependency in system while running with -T switch at /dev/fd/ +4 line 61, <GEN1> line 5.` [download] 15:21:32 BST update $ ~eeivob05/bin/locklbtype -vob /vobs/atcctest -l MG -u Tainted(eaccount): 0, eeivob05 Tainted(log): 0, /home/eeivob05/RANOS_autobuild/builds/logs/lbunlock.l +og Tainted(owner): 1, vobadm13 Tainted(uowner): 0, vobadm13 Tainted(op): 0, unlock Tainted(lbtype): 0, lbtype:MG@/vobs/atcctest Tainted(ssh): 0, /usr/bin/ssh Tainted(host): 0, eieatx008 Tainted(binct): 0, /opt/rational/clearcase/bin/cleartool Insecure dependency in system while running with -T switch at /dev/fd/ +4 line 78, <GEN1> line 5. $ ct diff -diff -pred locklbtype 8,9c8,9 < use IPC::System::Simple; < use autodie; --- > # use IPC::System::Simple; > use Scalar::Util qw(tainted); 60,62c60,78 < $owner =~ s/[^-\@\w.]//g; < system($ssh, '-l', $owner, $host, $binct, @op, @lbtype); < map{print"$_\n"}($ssh, '-l', $owner, $host, $binct, @op, @lbtype); --- > my @untaintedbits; > foreach (split //, $owner) { > if (/([-\@\w.])/) { > push @untaintedbits, $1; > } > } > my $uowner = join '', @untaintedbits; > > print "Tainted(eaccount): ", tainted($eaccount), ", $eaccount\n"; > print "Tainted(log): ", tainted($log), ", $log\n"; > print "Tainted(owner): ", tainted($owner), ", $owner\n"; > print "Tainted(uowner): ", tainted($uowner), ", $uowner\n"; > print "Tainted(op): ", tainted(@op), ", @op\n"; > print "Tainted(lbtype): ", tainted(@lbtype), ", @lbtype\n"; > print "Tainted(ssh): ", tainted($ssh), ", $ssh\n"; > print "Tainted(host): ", tainted($host), ", $host\n"; > print "Tainted(binct): ", tainted($binct), ", $binct\n"; > # map{print"$_\n"}($ssh, '-l', $owner, $uowner, $host, $binct, @op, +@lbtype); > system($ssh, '-l', $uowner, $host, $binct, @op, @lbtype); [download] Sep 11 17:43 update $ ~eeivob05/bin/locklbtype -vob /vobs/atcctest -l MG -u Tainted(eaccount): 0, eeivob05 Tainted(log): 0, /home/eeivob05/RANOS_autobuild/builds/logs/lbunlock.l +og Tainted(owner): 1, vobadm13 Tainted(uowner): 0, vobadm13 Tainted(op): 0, unlock Tainted(lbtype): 0, lbtype:MG@/vobs/atcctest Tainted(ssh): 0, /usr/bin/ssh Tainted(host): 0, eieatx008 Tainted(binct): 0, /opt/rational/clearcase/bin/cleartool IPC::System::Simple::run called with tainted argument "lbtype:MG@/vobs +/atcctest" at (eval 10) line 13 at /dev/fd/4 line 79 [download] Sep 11 17:54:59 update $ ct diff -diff -pred locklbtype 61,64c61,67 < my @untaintedbits; < foreach (split //, $owner) { < if (/([-\@\w.])/) { < push @untaintedbits, $1; --- > sub untaint($) { > my $tainted = shift; > my @untaintedbits; > foreach (split //, $tainted) { > if (m%([-\@\w.:/])%) { > push @untaintedbits, $1; > } 65a69 > return join '', @untaintedbits; 67c71,72 < my $uowner = join '', @untaintedbits; --- > my $uowner = untaint($owner); > map { $_ = untaint($_) } @lbtype; $ ~eeivob05/bin/locklbtype -vob /vobs/atcctest -l MG -u Tainted(eaccount): 0, eeivob05 Tainted(log): 0, /home/eeivob05/RANOS_autobuild/builds/logs/lbunlock.l +og Tainted(owner): 1, vobadm13 Tainted(uowner): 0, vobadm13 Tainted(op): 0, unlock Tainted(lbtype): 0, lbtype:MG@/vobs/atcctest Tainted(ssh): 0, /usr/bin/ssh Tainted(host): 0, eieatx008 Tainted(binct): 0, /opt/rational/clearcase/bin/cleartool Unlocked label type "MG". [download]	[reply] [d/l] [select]
Re^3: Insecure dependency in system under -T, with list form invocation by scorpio17 (Canon) on Sep 11, 2008 at 13:43 UTC
You should read the docs on taint mode more carefully. The part you need is this: `# allow alphanumerics, period, hyphen, ampersand if ($data =~ /^([-\@\w.]+)$/) { # $data is tainted $data = $1; # $data now untainted } else { die "Bad data in '$data'"; # log this somewhere }` [download] The regex alone will NOT untaint the data - you must copy it through a capture variable, like $1, to untaint it.	[reply] [d/l]
Re^4: Insecure dependency in system under -T, with list form invocation by cramdorgi (Acolyte) on Sep 11, 2008 at 14:25 UTC
Thanks. I did in the meanwhile, and succeeded in untainting my `$owner` piece of data. But I still get the error... Marc	[reply] [d/l]
Re^5: Insecure dependency in system under -T, with list form invocation by mr_mischief (Monsignor) on Sep 11, 2008 at 16:00 UTC
Re^6: Insecure dependency in system under -T, with list form invocation by cramdorgi (Acolyte) on Sep 12, 2008 at 08:24 UTC
Some notes below your chosen depth have not been shown here

In Section Seekers of Perl Wisdom

15:21:32 BST update

Sep 11 17:43 update

Sep 11 17:54:59 update