system() forking before taint check?

p5pRT commented 22 years ago

Migrated from rt.perl.org#8465 (status was 'resolved')

Searchable as RT8465$

p5pRT commented 22 years ago

From rick.delaney@rogers.com

This is a bug report for perl from rick.delaney@rogers.com\, generated with the help of perlbug 1.33 running under perl v5.6.1.

[96]% perl -Tle 'print "$$:"\, eval { system "ls $ENV{PATH}";$$ }' 9530: [97]% perl -Tle 'print "$$:"\, eval { system 'ls'\, $ENV{PATH};$$ }' 9531: 9531:9531

I haven't looked at the source at all but it looks like the second form of system() is forking before it does its taint checking. Worse\, the parent continues past the failed system() to execute more code.

Flags: category=core severity=medium

Site configuration information for perl v5.6.1:

Configured by rick at Sat Nov 10 17:49:00 EST 2001.

Summary of my perl5 (revision 5.0 version 6 subversion 1) configuration: Platform: osname=linux\, osvers=2.4.2-2\, archname=i686-linux uname='linux cs839290-a 2.4.2-2 #1 sun apr 8 20:41:30 edt 2001 i686 unknown ' config_args='-Doptimize=-g' hint=recommended\, useposix=true\, d_sigaction=define usethreads=undef use5005threads=undef useithreads=undef usemultiplicity=undef useperlio=undef d_sfio=undef uselargefiles=define usesocks=undef use64bitint=undef use64bitall=undef uselongdouble=undef Compiler: cc='cc'\, ccflags ='-DDEBUGGING -fno-strict-aliasing -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64'\, optimize='-g'\, cppflags='-DDEBUGGING -fno-strict-aliasing' ccversion=''\, gccversion='2.96 20000731 (Red Hat Linux 7.1 2.96-81)'\, gccosandvers='' intsize=4\, longsize=4\, ptrsize=4\, doublesize=8\, byteorder=1234 d_longlong=define\, longlongsize=8\, d_longdbl=define\, longdblsize=12 ivtype='long'\, ivsize=4\, nvtype='double'\, nvsize=8\, Off_t='off_t'\, lseeksize=8 alignbytes=4\, usemymalloc=n\, prototype=define Linker and Libraries: ld='cc'\, ldflags =' -L/usr/local/lib' libpth=/usr/local/lib /lib /usr/lib libs=-lnsl -lndbm -lgdbm -ldl -lm -lc -lcrypt -lutil perllibs=-lnsl -ldl -lm -lc -lcrypt -lutil libc=/lib/libc-2.2.2.so\, so=so\, useshrplib=false\, libperl=libperl.a Dynamic Linking: dlsrc=dl_dlopen.xs\, dlext=so\, d_dlsymun=undef\, ccdlflags='-rdynamic' cccdlflags='-fpic'\, lddlflags='-shared -L/usr/local/lib'

Locally applied patches:

@INC for perl v5.6.1: /usr/local/lib/perl5/5.6.1/i686-linux /usr/local/lib/perl5/5.6.1 /usr/local/lib/perl5/site_perl/5.6.1/i686-linux /usr/local/lib/perl5/site_perl/5.6.1 /usr/local/lib/perl5/site_perl .

Environment for perl v5.6.1: HOME=/home/rick LANG=en_US LANGUAGE (unset) LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=/usr/bin:/bin:/usr/X11R6/bin:/usr/local/bin:/opt/bin:/usr/bin:/home/rick/bin:/home/rick/bin PERL_BADLANG (unset) SHELL=/bin/zsh

p5pRT commented 22 years ago

From [Unknown Contact. See original ticket]

I can't follow up to my original report because it never reached my mailbox. But it's worse than I thought since multi-arg system and exec always succeed under -T.

Some more data points:

Single arg system: % perl -Te 'system "@ARGV"' /bin/echo not ok Insecure $ENV{PATH} while running with -T switch at -e line 1. % perl -Te '%ENV=();system "@ARGV"' /bin/echo not ok Insecure dependency in system while running with -T switch at -e line 1.

Multi arg system: % perl -Te 'system @ARGV' /bin/echo not ok
not ok % perl -Te '%ENV=();system @ARGV' /bin/echo not ok not ok

Single arg exec: % perl -Te 'exec "@ARGV"' /bin/echo not ok
Insecure $ENV{PATH} while running with -T switch at -e line 1. % perl -Te '%ENV=();exec "@ARGV"' /bin/echo not ok Insecure dependency in exec while running with -T switch at -e line 1.

Multi arg exec: % perl -Te 'exec @ARGV' /bin/echo not ok
not ok % perl -Te '%ENV=();exec @ARGV' /bin/echo not ok not ok

It's not just @ARGV. Any tainted data will happily be executed.

-- Rick Delaney rick.delaney@rogers.com

p5pRT commented 22 years ago

From [Unknown Contact. See original ticket]

Rick Delaney \rick\.delaney@rogers\.com wrote

I can't follow up to my original report because it never reached my mailbox. But it's worse than I thought since multi-arg system and exec always succeed under -T.

As documented. From perlsec:

Tainted data may not be used directly or indirectly in any command that invokes a sub-shell\, nor in any command that modifies files\, directories\, or processes\, with the following exceptions:

o If you pass a list of arguments to either "system" or "exec"\, the elements of that list are not checked for taintedness.

Mike Guy

p5pRT commented 22 years ago

From @schwern

On Tue\, Feb 12\, 2002 at 06:37:23PM +0000\, Mike Guy wrote:

Rick Delaney \rick\.delaney@rogers\.com wrote

I can't follow up to my original report because it never reached my mailbox. But it's worse than I thought since multi-arg system and exec always succeed under -T.

As documented. From perlsec:
             Tainted data may not be used directly or
 indirectly in any command that invokes a sub\-shell\, nor in
 any command that modifies files\, directories\, or processes\,
 with the following exceptions&#8203;:

 o   If you pass a list of arguments to either "system" or
     "exec"\, the elements of that list are not checked for
     taintedness\.

This behavior has changed a bit very recently in bleadperl. The some of Rick's examples will now fail under taint. Lemme compile up a more recent bleadperl and give details.

--

Michael G. Schwern \schwern@pobox\.com http://www.pobox.com/~schwern/ Perl Quality Assurance \perl\-qa@perl\.org Kwalitee Is Job One stretch your colon out\, put some effort into it\, and shit through that paste. -- japhy

p5pRT commented 22 years ago

From [Unknown Contact. See original ticket]

Michael G Schwern \schwern@pobox\.com writes:

On Tue\, Feb 12\, 2002 at 06:37:23PM +0000\, Mike Guy wrote:
Rick Delaney \rick\.delaney@rogers\.com wrote

I can't follow up to my original report because it never reached my mailbox. But it's worse than I thought since multi-arg system and exec always succeed under -T.

As documented. From perlsec:
             Tainted data may not be used directly or
 indirectly in any command that invokes a sub\-shell\, nor in
 any command that modifies files\, directories\, or processes\,
 with the following exceptions&#8203;:

 o   If you pass a list of arguments to either "system" or
     "exec"\, the elements of that list are not checked for
     taintedness\.

Wow\, thanks\, not sure how I missed that. I guess I just couldn't believe my eyes. What is the purpose of this "feature"? Its implementation just leads to inconsistencies. Look:

% perl -Tle 'eval { system "@ARGV" };print "\$\@=$@"' /bin/echo hello $@=Insecure $ENV{PATH} while running with -T switch at -e line 1.

% perl -Tle 'eval { system @ARGV };print "\$\@=$@"' /bin/echo hello hello $@=

Ok\, fine\, the second doesn't taint as documented.

% perl -Tle 'eval { system "@ARGV" };print "\$\@=$@"' echo hello $@=Insecure $ENV{PATH} while running with -T switch at -e line 1.

% perl -Tle 'eval { system @ARGV };print "\$\@=$@"' echo hello $@=Insecure $ENV{PATH} while running with -T switch at -e line 1.

$@=

But the second does TAINT_ENV in this example. It is inconsistent\, not even mentioning that it prints twice which is obviously wrong.

This behavior has changed a bit very recently in bleadperl. The some of Rick's examples will now fail under taint. Lemme compile up a more recent bleadperl and give details.

Great\, thanks!

-- Rick Delaney rick.delaney@rogers.com

p5pRT commented 22 years ago

From @schwern

On Tue\, Feb 12\, 2002 at 12:02:59AM -0500\, Rick Delaney wrote:

I can't follow up to my original report because it never reached my mailbox. But it's worse than I thought since multi-arg system and exec always succeed under -T.

Some more data points:

Here's what it looks like as of 14663

Single arg system: % perl -Te 'system "@ARGV"' /bin/echo not ok Insecure $ENV{PATH} while running with -T switch at -e line 1. % perl -Te '%ENV=();system "@ARGV"' /bin/echo not ok Insecure dependency in system while running with -T switch at -e line 1.

Same.

Multi arg system: % perl -Te 'system @ARGV' /bin/echo not ok
not ok

Now "Insecure $ENV{PATH} while running with -T switch at -e line 1."

% perl -Te '%ENV=();system @ARGV' /bin/echo not ok not ok

Same.

Single arg exec: % perl -Te 'exec "@ARGV"' /bin/echo not ok
Insecure $ENV{PATH} while running with -T switch at -e line 1. % perl -Te '%ENV=();exec "@ARGV"' /bin/echo not ok Insecure dependency in exec while running with -T switch at -e line 1.

Same.

Multi arg exec: % perl -Te 'exec @ARGV' /bin/echo not ok
not ok % perl -Te '%ENV=();exec @ARGV' /bin/echo not ok not ok

Same.

--

Michael G. Schwern \schwern@pobox\.com http://www.pobox.com/~schwern/ Perl Quality Assurance \perl\-qa@perl\.org Kwalitee Is Job One Still not king

p5pRT commented 22 years ago

From @schwern

On Tue\, Feb 12\, 2002 at 07:36:52PM -0500\, Rick Delaney wrote:

As documented. From perlsec:
             Tainted data may not be used directly or
 indirectly in any command that invokes a sub\-shell\, nor in
 any command that modifies files\, directories\, or processes\,
 with the following exceptions&#8203;:

 o   If you pass a list of arguments to either "system" or
     "exec"\, the elements of that list are not checked for
     taintedness\.
Wow\, thanks\, not sure how I missed that. I guess I just couldn't believe my eyes. What is the purpose of this "feature"? Its implementation just leads to inconsistencies. Look: \

That information was orignally added to perlsec between 5.004_04 and 5.004_05. It looks like it was in change 904.

http://www.xray.mpe.mpg.de/mailing-lists/perl5-porters/1998-04/msg01378.html

Title: "(repost) new text for perlsec"\, "new text for perlsec" From: Tom Phoenix \rootbeer@teleport\.com Msg-ID: \Pine\.GSO\.3\.96\.980423161605\.5518N\-100000@user2\.teleport\.com Files: pod/perlsec.pod

From there you should be able to track down the reason and see if it still makes sense.

--

Michael G. Schwern \schwern@pobox\.com http://www.pobox.com/~schwern/ Perl Quality Assurance \perl\-qa@perl\.org Kwalitee Is Job One "You killed my fish?" "Why does that pickle you?" http://sluggy.com/d/010204.html

p5pRT commented 22 years ago

From [Unknown Contact. See original ticket]

Michael G Schwern \schwern@pobox\.com writes:

On Tue\, Feb 12\, 2002 at 07:36:52PM -0500\, Rick Delaney wrote:
As documented. From perlsec:
             Tainted data may not be used directly or
 indirectly in any command that invokes a sub\-shell\, nor in
 any command that modifies files\, directories\, or processes\,
 with the following exceptions&#8203;:

 o   If you pass a list of arguments to either "system" or
     "exec"\, the elements of that list are not checked for
     taintedness\.
Wow\, thanks\, not sure how I missed that. I guess I just couldn't believe my eyes. What is the purpose of this "feature"? Its implementation just leads to inconsistencies. Look: \
That information was orignally added to perlsec between 5.004_04 and 5.004_05. It looks like it was in change 904.

http://www.xray.mpe.mpg.de/mailing-lists/perl5-porters/1998-04/msg01378.html
     Title&#8203;:  "$repost$ new text for perlsec"\, "new text for perlsec"
      From&#8203;:  Tom Phoenix \<rootbeer@&#8203;teleport\.com>
    Msg\-ID&#8203;:  \<Pine\.GSO\.3\.96\.980423161605\.5518N\-100000@&#8203;user2\.teleport\.com>
     Files&#8203;:  pod/perlsec\.pod
From there you should be able to track down the reason and see if it still makes sense.

Thanks\, the originating thread is:

http://www.xray.mpe.mpg.de/mailing-lists/perl5-porters/1998-03/msg01753.html

I'd guess that Ilya and Tom were both being smart-alecs and the sarcasm went over an overworked pumpking's head. I've cc'ed both in case they care to comment.

Is it too late for me to fix it? What's the statute of limitations on fixing documented taint bugs?

BTW\, if I make a patch it will just do the obvious thing and check all the args at the beginning of pp_system and pp_exec. If anyone has objections or better ideas\, please speak up.

-- Rick Delaney rick.delaney@rogers.com

p5pRT commented 22 years ago

From [Unknown Contact. See original ticket]

Rick Delaney wrote: [snip]

% perl -Tle 'eval { system @ARGV };print "\$\@=$@"' echo hello $@=Insecure $ENV{PATH} while running with -T switch at -e line 1.

$@=

But the second does TAINT_ENV in this example. It is inconsistent\, not even mentioning that it prints twice which is obviously wrong.

I think this might be caused by a fork before the taint check.

-- From the libwww-perl changelog\, describing an error fixed in 5.41: o The local/http.t test actually did try to unlink("."). This was very confusing on systems where it succeed.

p5pRT commented 22 years ago

From @schwern

On Tue\, Feb 12\, 2002 at 11:25:31PM -0500\, Rick Delaney wrote:

Thanks\, the originating thread is:

http://www.xray.mpe.mpg.de/mailing-lists/perl5-porters/1998-03/msg01753.html

I'd guess that Ilya and Tom were both being smart-alecs and the sarcasm went over an overworked pumpking's head. I've cc'ed both in case they care to comment.

Is it too late for me to fix it? What's the statute of limitations on fixing documented taint bugs?

Yeah\, it looks like we documented a bug. Since its a surprising and inconsistent bug\, and it involves taint\, I'd vote changing the behavior and the docs to do the right thing.

--

Michael G. Schwern \schwern@pobox\.com http://www.pobox.com/~schwern/ Perl Quality Assurance \perl\-qa@perl\.org Kwalitee Is Job One It's Miller time!

p5pRT commented 22 years ago

From [Unknown Contact. See original ticket]

Benjamin Goldberg \goldbb2@earthlink\.net writes:

Rick Delaney wrote: [snip]

% perl -Tle 'eval { system @ARGV };print "\$\@=$@"' echo hello $@=Insecure $ENV{PATH} while running with -T switch at -e line 1.

$@=

But the second does TAINT_ENV in this example. It is inconsistent\, not even mentioning that it prints twice which is obviously wrong.

I think this might be caused by a fork before the taint check.

Yes\, as I speculated in the original bug report. I am not the first to report this either (but mine has a bug id on it). Do you remember this Mike?

http://www.xray.mpe.mpg.de/mailing-lists/perl5-porters/1998-06/msg02215.html

It appears no one was listening then. I'll submit a patch on the weekend (or sooner if I find some spare tuits).

-- Rick Delaney rick.delaney@rogers.com

p5pRT commented 22 years ago

From [Unknown Contact. See original ticket]

On 12 Feb 2002 23:25:31 -0500\, Rick Delaney wrote (in part):

Rick> I'd guess that Ilya and Tom were both being smart-alecs and the Rick> sarcasm went over an overworked pumpking's head. I've cc'ed both in Rick> case they care to comment.

Rick> Is it too late for me to fix it? What's the statute of limitations on Rick> fixing documented taint bugs?

Rick> BTW\, if I make a patch it will just do the obvious thing and check all Rick> the args at the beginning of pp_system and pp_exec. If anyone has Rick> objections or better ideas\, please speak up.

'That's not a bug -- It's a feature.' Note that multi-argument exec() and system() do NOT invoke a shell on your behalf\, but that the single-argument forms do run that risk for you. The taint check is there for that "extra" shell. This is consistent with how tainting is handled in a lot of the rest of perl\, in that only some manipulations of tainted data are supposed to be denied by the "safety net".

Note also that it's been this way for a long time. Any attempt to "fix" this non-bug should run afoul of the backward-compatibility police. Unless\, of course\, you actually do so with a pragma so that the changed behaviour is only observed when requested.

-- Spider Boardman (at home) spider@Orb.Nashua.NH.US The management (my cats) made me say this. http://www.ultranet.com/~spiderb PGP public key fingerprint: 96 72 D2 C6 E0 92 32 89 F6 B2 C2 A0 1C AB 1F DC

p5pRT commented 22 years ago

From [Unknown Contact. See original ticket]

On Wed\, Feb 13\, 2002 at 12:36:49AM -0500\, Spider Boardman wrote:

Rick> BTW\, if I make a patch it will just do the obvious thing and check all Rick> the args at the beginning of pp_system and pp_exec. If anyone has Rick> objections or better ideas\, please speak up.

'That's not a bug -- It's a feature.' Note that multi-argument exec() and system() do NOT invoke a shell on your behalf\, but that the single-argument forms do run that risk for you.

Who cares? It is either/or: tainting is useful as far as it covers your ass. Currently it does not - and with the most dangerous operation\, startup of external programs.

The taint check is there for that "extra" shell.

Eh? What is the extra shell in

unlink $from_user_input;

?

This is consistent with how tainting is handled in a lot of the rest of perl\, in that only some manipulations of tainted data are supposed to be denied by the "safety net".

AFAIK\, there are only 2 ways to avoid tainting: $1 etc\, and system()/exec(). What exactly do you mean by "only some"

Note also that it's been this way for a long time.

*That's* the real problem\, not lame post-factum justifications of this insanity.

Any attempt to "fix" this non-bug should run afoul of the backward-compatibility police. Unless\, of course\, you actually do so with a pragma so that the changed behaviour is only observed when requested.

And this is almost as useless as the manual check of the data flow from the user input to syscalls... But I never got anything better than this...

use strict 'taint';

... Would not mind it switching off the untainting via REx too\, so that you need to explicitly

$filtered = untaint $input\, qr/^(\w+)\z/;

Yours\, Ilya

p5pRT commented 22 years ago

From [Unknown Contact. See original ticket]

On Wed\, 13 Feb 2002\, Spider Boardman wrote:

'That's not a bug -- It's a feature.' Note that multi-argument exec() and system() do NOT invoke a shell on your behalf\, but that the single-argument forms do run that risk for you. The taint check is there for that "extra" shell. This is consistent with how tainting is handled in a lot of the rest of perl\, in that only some manipulations of tainted data are supposed to be denied by the "safety net".

Note also that it's been this way for a long time. Any attempt to "fix" this non-bug should run afoul of the backward-compatibility police. Unless\, of course\, you actually do so with a pragma so that the changed behaviour is only observed when requested.

I agree. I've written more than one piece of code that _intentionally_ uses the fact that system(@array) doesn't check taint on arguments (and why should it? It has no damn idea what I intend to do with them and their values are not exposed to a shell which might attempt to execute them).

What's next? Tainting XS? After all - something wicked _just might_ be done inside. Tainting writes to filehandles because another program might use it in an unsafe way?

Breaking running code that _was_ written to strict\, warnings and taint without a _very_ compelling reason (ie direct exploit) is not a good thing. I judge from the production scripts I have seen 'in the wild' that people running taint in the first place are (admittedly disappointingly) a rare breed who already by and large understand the issues and can be trusted to have not done amazingly evil things with it inside the existing framework. The remainder mostly aren't even running tainted at all (and usually not strict or even warnings).

-- Jerry

Unix was not designed to stop people from doing stupid things\, because that would also stop them from doing clever things.

---Doug Gwyn

p5pRT commented 22 years ago

From @schwern

On Fri\, Feb 15\, 2002 at 08:34:08AM -0800\, Benjamin Franz wrote:

On Wed\, 13 Feb 2002\, Spider Boardman wrote:

'That's not a bug -- It's a feature.' Note that multi-argument exec() and system() do NOT invoke a shell on your behalf\, but that the single-argument forms do run that risk for you. The taint check is there for that "extra" shell. This is consistent with how tainting is handled in a lot of the rest of perl\, in that only some manipulations of tainted data are supposed to be denied by the "safety net".

Note also that it's been this way for a long time. Any attempt to "fix" this non-bug should run afoul of the backward-compatibility police. Unless\, of course\, you actually do so with a pragma so that the changed behaviour is only observed when requested.

I agree. I've written more than one piece of code that _intentionally_ uses the fact that system(@array) doesn't check taint on arguments (and why should it? It has no damn idea what I intend to do with them and their values are not exposed to a shell which might attempt to execute them).

I don't know where this started\, but the idea that taint is only for preventing things from being misinterpreted by the shell isn't quite right. Its just one of its responsibilities.

The overall responsibility of taint is to prevent you from accidentally changing the outside world in ways you didn't anticipate. It does this by not allowing you to use tainted variables on any operation which might effect the Universe Outside Your Program.

Some examples of tainting that have nothing to do with a shell:

open(FOO\, "> $tainted"); unlink $tainted; umask $tained;

If "system LIST" is 'secure' just because it doesn't use the shell then all the above should be thrown out.

The only reason "system LIST" and "exec LIST" should be left untainted would be for backwards compatibility reasons. I'd be disappointed if they were\, seeing as how system LIST is generally better than system EXPR but not if it means you're bypassing taint.

What's next? Tainting XS? After all - something wicked _just might_ be done inside. Tainting writes to filehandles because another program might use it in an unsafe way?

We already do the latter\, effectively\, by not allowing you to open a tainted file for writing. :) XS... we assume you know what you're doing if you're getting into XS (in reality\, its just impractical).

--

Michael G. Schwern \schwern@pobox\.com http://www.pobox.com/~schwern/ Perl Quality Assurance \perl\-qa@perl\.org Kwalitee Is Job One conway: unit of mind expansion. One Conway == ~20 lines of Perl code found in $CPAN/authors/id/D/DC/DCONWAY\, which gives the sensation of your brain being wrapped around a brick\, with kiwi juice squeezed on top. -- Ziggy

p5pRT commented 22 years ago

From [Unknown Contact. See original ticket]

The following patch makes it necessary to untaint LIST in `system LIST` and `exec LIST` since Perl has no way of knowing what an external program will do with its arguments--just like Perl doesn't know what it will do with its environment.

It also fixes this bug id by moving the taint check before the fork in pp_system.

-- Rick Delaney rick.delaney@rogers.com

Inline Patch

```diff diff -ruN perl-current/pod/perlsec.pod perl-patch/pod/perlsec.pod --- perl-current/pod/perlsec.pod Thu Sep 6 10:37:18 2001 +++ perl-patch/pod/perlsec.pod Sun Feb 17 00:05:35 2002 @@ -44,11 +44,6 @@ =item * -If you pass more than one argument to either C or C, -the arguments are B checked for taintedness. - -=item * - Arguments to C and C are B checked for taintedness. =back @@ -72,7 +67,8 @@ $data = 'abc'; # Not tainted system "echo $arg"; # Insecure - system "/bin/echo", $arg; # Secure (doesn't use sh) + system "/bin/echo", $arg; # Considered insecure + # (Perl doesn't know about /bin/echo) system "echo $hid"; # Insecure system "echo $data"; # Insecure until PATH set @@ -87,18 +83,18 @@ open(FOO, "< $arg"); # OK - read-only file open(FOO, "> $arg"); # Not OK - trying to write - open(FOO,"echo $arg|"); # Not OK, but... + open(FOO,"echo $arg|"); # Not OK open(FOO,"-|") - or exec 'echo', $arg; # OK + or exec 'echo', $arg; # Also not OK $shout = `echo $arg`; # Insecure, $shout now tainted unlink $data, $arg; # Insecure umask $arg; # Insecure - exec "echo $arg"; # Insecure (uses the shell) - exec "echo", $arg; # Secure (doesn't use the shell) - exec "sh", '-c', $arg; # Considered secure, alas! + exec "echo $arg"; # Insecure + exec "echo", $arg; # Insecure + exec "sh", '-c', $arg; # Very insecure @files = <*.c>; # insecure (uses readdir() or similar) @files = glob('*.c'); # insecure (uses readdir() or similar) @@ -112,9 +108,7 @@ $arg, `true`; # Insecure (although it isn't really) If you try to do something insecure, you will get a fatal error saying -something like "Insecure dependency" or "Insecure $ENV{PATH}". Note that you -can still write an insecure B or B, but only by explicitly -doing something like the "considered secure" example above. +something like "Insecure dependency" or "Insecure $ENV{PATH}". =head2 Laundering and Detecting Tainted Data diff -ruN perl-current/pp_sys.c perl-patch/pp_sys.c --- perl-current/pp_sys.c Thu Feb 14 18:00:18 2002 +++ perl-patch/pp_sys.c Sat Feb 16 23:40:48 2002 @@ -4031,12 +4031,15 @@ int pp[2]; I32 did_pipes = 0; - if (SP - MARK == 1) { - if (PL_tainting) { - (void)SvPV_nolen(TOPs); /* stringify for taint check */ - TAINT_ENV(); - TAINT_PROPER("system"); + if (PL_tainting) { + TAINT_ENV(); + while (++MARK <= SP) { + (void)SvPV_nolen(*MARK); /* stringify for taint check */ + if (PL_tainted) + break; } + MARK = ORIGMARK; + TAINT_PROPER("system"); } PERL_FLUSHALL_FOR_CHILD; #if (defined(HAS_FORK) || defined(AMIGAOS)) && !defined(VMS) && !defined(OS2) || defined(PERL_MICRO) @@ -4044,16 +4047,6 @@ Pid_t childpid; int status; Sigsave_t ihand,qhand; /* place to save signals during system() */ - - if (PL_tainting) { - SV *cmd = NULL; - if (PL_op->op_flags & OPf_STACKED) - cmd = *(MARK + 1); - else if (SP - MARK != 1) - cmd = *SP; - if (cmd && *(SvPV_nolen(cmd)) != '/') - TAINT_ENV(); - } if (PerlProc_pipe(pp) >= 0) did_pipes = 1; @@ -4155,6 +4148,16 @@ I32 value; STRLEN n_a; + if (PL_tainting) { + TAINT_ENV(); + while (++MARK <= SP) { + (void)SvPV_nolen(*MARK); /* stringify for taint check */ + if (PL_tainted) + break; + } + MARK = ORIGMARK; + TAINT_PROPER("exec"); + } PERL_FLUSHALL_FOR_CHILD; if (PL_op->op_flags & OPf_STACKED) { SV *really = *++MARK; @@ -4174,11 +4177,6 @@ # endif #endif else { - if (PL_tainting) { - (void)SvPV_nolen(*SP); /* stringify for taint check */ - TAINT_ENV(); - TAINT_PROPER("exec"); - } #ifdef VMS value = (I32)vms_do_exec(SvPVx(sv_mortalcopy(*SP), n_a)); #else diff -ruN perl-current/t/op/taint.t perl-patch/t/op/taint.t --- perl-current/t/op/taint.t Fri Feb 8 10:12:27 2002 +++ perl-patch/t/op/taint.t Sun Feb 17 00:16:01 2002 @@ -119,7 +119,7 @@ close PROG; my $echo = "$Invoke_Perl $ECHO"; -print "1..183\n"; +print "1..203\n"; # First, let's make sure that Perl is checking the dangerous # environment variables. Maybe they aren't set yet, so we'll @@ -926,4 +926,29 @@ local $ENV{PATH} .= $TAINT; eval { system { "echo" } "/arg0", "arg1" }; test 183, $@ =~ /^Insecure \$ENV/; +} +{ + # bug 20020208.005 plus some extras + # single arg exec/system are tests 80-83 + test 184, eval { exec $TAINT, $TAINT } eq '', 'exec'; + test 185, $@ =~ /^Insecure dependency/, $@; + test 186, eval { exec $TAINT $TAINT } eq '', 'exec'; + test 187, $@ =~ /^Insecure dependency/, $@; + test 188, eval { exec $TAINT $TAINT, $TAINT } eq '', 'exec'; + test 189, $@ =~ /^Insecure dependency/, $@; + test 190, eval { exec $TAINT 'notaint' } eq '', 'exec'; + test 191, $@ =~ /^Insecure dependency/, $@; + test 192, eval { exec {'notaint'} $TAINT } eq '', 'exec'; + test 193, $@ =~ /^Insecure dependency/, $@; + + test 194, eval { system $TAINT, $TAINT } eq '', 'system'; + test 195, $@ =~ /^Insecure dependency/, $@; + test 196, eval { system $TAINT $TAINT } eq '', 'exec'; + test 197, $@ =~ /^Insecure dependency/, $@; + test 198, eval { system $TAINT $TAINT, $TAINT } eq '', 'exec'; + test 199, $@ =~ /^Insecure dependency/, $@; + test 200, eval { system $TAINT 'notaint' } eq '', 'exec'; + test 201, $@ =~ /^Insecure dependency/, $@; + test 202, eval { system {'notaint'} $TAINT } eq '', 'exec'; + test 203, $@ =~ /^Insecure dependency/, $@; } ```

p5pRT commented 22 years ago

From [Unknown Contact. See original ticket]

The following patch makes it necessary to untaint LIST in `system LIST` and `exec LIST` since Perl has no way of knowing what an external program will do with its arguments--just like Perl doesn't know what it will do with its environment.

It also fixes this bug id by moving the taint check before the fork in pp_system.

-- Rick Delaney rick.delaney@rogers.com

Since there is no consensus on the system LIST form could you please split the patch into one that fixes the bug and one that tainchecks LIST.

Arthur

p5pRT commented 22 years ago

From @jhi

On Sun\, Feb 17\, 2002 at 10:58:31AM +0100\, Arthur Bergman wrote:

The following patch makes it necessary to untaint LIST in `system LIST` and `exec LIST` since Perl has no way of knowing what an external program will do with its arguments--just like Perl doesn't know what it will do with its environment.

It also fixes this bug id by moving the taint check before the fork in pp_system.

-- Rick Delaney rick.delaney@rogers.com

Since there is no consensus on the system LIST form could you please split the patch into one that fixes the bug and one that tainchecks LIST.

Agreed. Changing taint behaviour so greatly is one of those "Larry shall decide" issues. (I have a list of such issues I will pass by Larry after 5.7.3 and before 5.8.0 is minted.) But as a first cut\, the taint behaviour change will NOT go in.

Arthur

-- $jhi++; # http://www.iki.fi/jhi/ # There is this special biologist word we use for 'stable'. # It is 'dead'. -- Jack Cohen

p5pRT commented 22 years ago

From [Unknown Contact. See original ticket]

On Sun\, Feb 17\, 2002 at 10:58:31AM +0100\, Arthur Bergman wrote:

The following patch makes it necessary to untaint LIST in `system LIST` and `exec LIST` since Perl has no way of knowing what an external program will do with its arguments--just like Perl doesn't know what it will do with its environment.

It also fixes this bug id by moving the taint check before the fork in pp_system.

-- Rick Delaney rick.delaney@rogers.com

Since there is no consensus on the system LIST form could you please split the patch into one that fixes the bug and one that tainchecks LIST.

Agreed. Changing taint behaviour so greatly is one of those "Larry shall decide" issues. (I have a list of such issues I will pass by Larry after 5.7.3 and before 5.8.0 is minted.) But as a first cut\, the taint behaviour change will NOT go in.

Ok\, but the part which fixes the taint before the fork check has no controversy with it? if not patch + test cases would be very welcome.

Arthur

p5pRT commented 22 years ago

From @jhi

On Sun\, Feb 17\, 2002 at 07:38:59PM +0100\, Arthur Bergman wrote:

On Sun\, Feb 17\, 2002 at 10:58:31AM +0100\, Arthur Bergman wrote:

The following patch makes it necessary to untaint LIST in `system LIST` and `exec LIST` since Perl has no way of knowing what an external program will do with its arguments--just like Perl doesn't know what it will do with its environment.

It also fixes this bug id by moving the taint check before the fork in pp_system.

-- Rick Delaney rick.delaney@rogers.com

Since there is no consensus on the system LIST form could you please split the patch into one that fixes the bug and one that tainchecks LIST.

Agreed. Changing taint behaviour so greatly is one of those "Larry shall decide" issues. (I have a list of such issues I will pass by Larry after 5.7.3 and before 5.8.0 is minted.) But as a first cut\, the taint behaviour change will NOT go in.

Ok\, but the part which fixes the taint before the fork check has no controversy with it? if not patch + test cases would be very welcome.

I think that part is okay.

Arthur

-- $jhi++; # http://www.iki.fi/jhi/ # There is this special biologist word we use for 'stable'. # It is 'dead'. -- Jack Cohen

p5pRT commented 20 years ago

From rick@bort.ca

All the patches for this bug were eventually applied. Closing.

p5pRT commented 20 years ago

rick@bort.ca - Status changed from 'open' to 'resolved'

Perl / perl5