no taint checks on symbolic references

p5pRT commented 22 years ago

Migrated from rt.perl.org#8928 (status was 'stalled')

Searchable as RT8928$

p5pRT commented 22 years ago

From ota-11@andrew.pimlott.net

(I tried querying for this on bugs.perl.com\, but I'm not sure it was working\, because it never returned any hits.)

The following runs without warning:

#!/usr/bin/perl -T

$ENV{PATH} = '/bin'; chomp($foo = \<>); # $foo is tainted &$foo;

sub iownyou { print "Game over\n" }

If you type in "iownyou"\, it runs &iownyou. Taint checking has no effect on symbolic dereferences.

The idea is from a Phrack article: http://www.phrack.com/show.php?p=58&a=9

Andrew

Perl Info

``` Flags: category=core severity=medium Site configuration information for perl v5.6.1: Configured by bod at Fri Jan 11 04:14:18 EST 2002. Summary of my perl5 (revision 5.0 version 6 subversion 1) configuration: Platform: osname=linux, osvers=2.4.13, archname=i386-linux uname='linux duende 2.4.13 #1 wed oct 31 19:18:07 est 2001 i686 unknown ' config_args='-Dccflags=-DDEBIAN -Dcccdlflags=-fPIC -Darchname=i386-linux -Dprefix=/usr -Dprivlib=/usr/share/perl/5.6.1 -Darchlib=/usr/lib/perl/5.6.1 -Dvendorprefix=/usr -Dvendorlib=/usr/share/perl5 -Dvendorarch=/usr/lib/perl5 -Dsiteprefix=/usr/local -Dsitelib=/usr/local/share/perl/5.6.1 -Dsitearch=/usr/local/lib/perl/5.6.1 -Dman1dir=/usr/share/man/man1 -Dman3dir=/usr/share/man/man3 -Dman1ext=1 -Dman3ext=3perl -Dpager=/usr/bin/sensible-pager -Uafs -Ud_csh -Uusesfio -Duseshrplib -Dlibperl=libperl.so.5.6.1 -Dd_dosuid -des' 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 ='-DDEBIAN -fno-strict-aliasing -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64', optimize='-O2', cppflags='-DDEBIAN -fno-strict-aliasing -I/usr/local/include' ccversion='', gccversion='2.95.4 (Debian prerelease)', 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=-lgdbm -ldb -ldl -lm -lc -lcrypt perllibs=-ldl -lm -lc -lcrypt libc=/lib/libc-2.2.4.so, so=so, useshrplib=true, libperl=libperl.so.5.6.1 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: /home/pimlott/local/lib/perl5 /usr/local/lib/perl/5.6.1 /usr/local/share/perl/5.6.1 /usr/lib/perl5 /usr/share/perl5 /usr/lib/perl/5.6.1 /usr/share/perl/5.6.1 /usr/local/lib/site_perl . Environment for perl v5.6.1: HOME=/home/pimlott LANG=C LANGUAGE (unset) LC_CTYPE=en_US LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=/home/pimlott/bin:/usr/local/atria/bin:/home/pimlott/local/jdk1.3/bin:/home/pimlott/local/j2sdkee1.2.1/bin:/usr/sbin:/sbin:/home/pimlott/bin:/usr/local/atria/bin:/home/pimlott/local/jdk1.3/bin:/home/pimlott/local/j2sdkee1.2.1/bin:/usr/sbin:/sbin:/usr/local/bin:/usr/bin:/bin:/usr/bin/X11:/usr/games PERL5LIB=/home/pimlott/local/lib/perl5 PERL_BADLANG (unset) SHELL=/bin/zsh ```

p5pRT commented 22 years ago

From @jhi

On Thu\, Apr 11\, 2002 at 04:00:24PM -0400\, Andrew Pimlott wrote:

This is a bug report for perl generated with the help of perlbug 1.33 running under perl v5.6.1.

----------------------------------------------------------------- [Please enter your report here]

(I tried querying for this on bugs.perl.com\, but I'm not sure it was working\, because it never returned any hits.)

The following runs without warning:
\#\!/usr/bin/perl \-T

$ENV\{PATH\} = '/bin';
chomp$$foo = \<>$;  \# $foo is tainted
&$foo;

sub iownyou \{ print "Game over\\n" \}
If you type in "iownyou"\, it runs &iownyou. Taint checking has no effect on symbolic dereferences.

This is not so much different [1] from

$ENV{PATH} = '/bin'; chomp($foo = \<>); # $foo is tainted sub iownyou { print "Game over\n" } if ($foo) { iownyou }

In other words\, taint does not affect control flow.

[1] Symbolic references is of course "powerful" since it gives you more power to alter the control flow. But it's still control flow\, nevertheless.

The idea is from a Phrack article: http://www.phrack.com/show.php?p=58&a=9

Everybody seems to be finding this :-) (I did few days ago)

I quote from perlsec:

... taint mode ... You may not use data derived from outside your program to affect something else outside your program--at least\, not by accident.
In other words\, taint isn't "if tainted\, do nothing". It is "if tainted\, spread taint no further".

Months ago there was discussion about this\, but the end result was that changing the semantics of taint to be that much more stringent would require some serious consulting with Larry\, and besides\, Perl 5.8.0 was close. Now it is even closer.

-- $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 @jhi

On Thu\, Apr 11\, 2002 at 04:33:06PM -0400\, Andrew Pimlott wrote:

On Thu\, Apr 11\, 2002 at 11:32:15PM +0300\, Jarkko Hietaniemi wrote:

In other words\, taint does not affect control flow.

[1] Symbolic references is of course "powerful" since it gives you more power to alter the control flow. But it's still control flow\, nevertheless. ... ... taint mode ... You may not use data derived from outside your program to affect something else outside your program--at least\, not by accident.

This is a good explanation and I buy it. I would only add that It Would Be Nice to have a variant on taint mode that is designed to prevent tainted data from affecting control flow.

Something to ponder for 5.9.

Andrew

-- $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]

Jarkko Hietaniemi wrote:

On Thu\, Apr 11\, 2002 at 04:00:24PM -0400\, Andrew Pimlott wrote:
This is a bug report for perl generated with the help of perlbug 1.33 running under perl v5.6.1.

----------------------------------------------------------------- [Please enter your report here]

(I tried querying for this on bugs.perl.com\, but I'm not sure it was working\, because it never returned any hits.)

The following runs without warning:
\#\!/usr/bin/perl \-T

$ENV\{PATH\} = '/bin';
chomp$$foo = \<>$;  \# $foo is tainted
&$foo;

sub iownyou \{ print "Game over\\n" \}
If you type in "iownyou"\, it runs &iownyou. Taint checking has no effect on symbolic dereferences.
This is not so much different [1] from
$ENV\{PATH\} = '/bin';
chomp$$foo = \<>$;  \# $foo is tainted
sub iownyou \{ print "Game over\\n" \}
if $$foo$ \{ iownyou \}
In other words\, taint does not affect control flow.

[1] Symbolic references is of course "powerful" since it gives you more power to alter the control flow. But it's still control flow\, nevertheless.

What about something like this:

chomp(my ($class\, $meth\, @args) = \<>); $class->$meth($args);

And the user types in: rm POSIX::system -rf / ^D

Is this still merely flow control?

The idea is from a Phrack article: http://www.phrack.com/show.php?p=58&a=9

Everybody seems to be finding this :-) (I did few days ago)

I quote from perlsec:

... taint mode ... You may not use data derived from outside your program to affect something else outside your program--at least\, not by accident.

In other words\, taint isn't "if tainted\, do nothing". It is "if tainted\, spread taint no further".

Months ago there was discussion about this\, but the end result was that changing the semantics of taint to be that much more stringent would require some serious consulting with Larry\, and besides\, Perl 5.8.0 was close. Now it is even closer.

Changing perl to prevent use of tainted method names or symbolic reference sub calls might not be doable in the given time frame\, but perhaps there's time to add a warning to the documentation?

Hmm\, how to phrase it\, though...

-- print reverse( "\,rekcah"\, " lreP"\, " rehtona"\, " tsuJ" )."\n";

p5pRT commented 22 years ago

From @jhi

What about something like this:

chomp(my ($class\, $meth\, @args) = \<>); $class->$meth($args);

And the user types in: rm POSIX::system -rf / ^D

Is this still merely flow control?

Yes. Look it up.

Do not think that I'm saying this is an innocent or insignificant problem. I just do not see any simple ways to fix it\, especially given the time frame. While it's a definite hole in the taint system\, but all the fuss seems to be a bit blown out of proportion\, maybe because the problem has been witnessed in a currently fashionable and buzzword-compliant piece of software.

Taint is not a silver bullet that completely and instantly and permanently secures your code: it's tool that can help in doing that\, and now a corner of that tool seemingly needs some sharpening.

Changing perl to prevent use of tainted method names or symbolic reference sub calls might not be doable in the given time frame\, but

Don't fool yourself by stopping at that. It must be all of control flow.

perhaps there's time to add a warning to the documentation?

Hmm\, how to phrase it\, though...

That's easy:

"Don't use symbolic method names or do symbolic reference sub calls."

The real question where to put it.

-- $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 Thu\, Apr 11\, 2002 at 11:32:15PM +0300\, Jarkko Hietaniemi wrote:

In other words\, taint does not affect control flow.

[1] Symbolic references is of course "powerful" since it gives you more power to alter the control flow. But it's still control flow\, nevertheless. .... ... taint mode ... You may not use data derived from outside your program to affect something else outside your program--at least\, not by accident.

This is a good explanation and I buy it. I would only add that It Would Be Nice to have a variant on taint mode that is designed to prevent tainted data from affecting control flow.

Andrew

p5pRT commented 22 years ago

From @abigail

On Thu\, Apr 11\, 2002 at 04:33:06PM -0400\, Andrew Pimlott wrote:

On Thu\, Apr 11\, 2002 at 11:32:15PM +0300\, Jarkko Hietaniemi wrote:

In other words\, taint does not affect control flow.

[1] Symbolic references is of course "powerful" since it gives you more power to alter the control flow. But it's still control flow\, nevertheless. .... ... taint mode ... You may not use data derived from outside your program to affect something else outside your program--at least\, not by accident.

This is a good explanation and I buy it. I would only add that It Would Be Nice to have a variant on taint mode that is designed to prevent tainted data from affecting control flow.

But\, as Jarkko says\, "don't use symbolic references or symbolic system calls"\, you're probably better off with a different kind of strictness. 'use strict "refs"' already prevents the first action - maybe it's possible to prevent the second action as well.

As for a different kind of taint preventing tainted data to affect control flow\, think how you want to untaint such tainted data. Now we typically do something like:

die "Dangerous data\n" unless $data =~ /^([a-z]+)$/; $data = $1;

But now you suddenly have tainted data affecting control flow....

Abigail

p5pRT commented 22 years ago

From @paulg1973

I suggest that the language about taint and symbolic references to into ../pod/perlsec.pod\, in the list of exceptions that begins around line 45. I see it as a new item in this list.

Thanks PG -- Paul Green\, Senior Technical Consultant\, Stratus Technologies. Voice: +1 978-461-7557; FAX: +1 978-461-3610; Video on request.

p5pRT commented 22 years ago

From @jhi

I consulted the Supreme Justice of The Camel and thus spake Judge Wall:

: Well\, I presume this might have prevented the SOAP::Lite fiasco? Seems : like a good argument for doing it. I don't think it'd be much overhead.

So I repent.

-- $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 @jhi

On Fri\, Apr 12\, 2002 at 07:05:58PM +0300\, Jarkko Hietaniemi wrote:

I consulted the Supreme Justice of The Camel and thus spake Judge Wall:

: Well\, I presume this might have prevented the SOAP::Lite fiasco? Seems : like a good argument for doing it. I don't think it'd be much overhead.

More context: I asked only about the symbolic method names and symbolic subref calls\, not about control flow in general.

So I repent.

Any takers for the patch?

-- $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 @jhi

On Fri\, Apr 12\, 2002 at 07:08:38PM +0300\, Jarkko Hietaniemi wrote:

On Fri\, Apr 12\, 2002 at 07:05:58PM +0300\, Jarkko Hietaniemi wrote:

I consulted the Supreme Justice of The Camel and thus spake Judge Wall:

: Well\, I presume this might have prevented the SOAP::Lite fiasco? Seems : like a good argument for doing it. I don't think it'd be much overhead.

More context: I asked only about the symbolic method names and symbolic subref calls\, not about control flow in general.

More: in Larry's opinion the larger issue of control flow is not actually larger: there's a decision made based on tainted data\, but it's a boolean decision\, as opposed to running arbitrary code (which the method and subroutine indirection allows).

Also\, an earlier quote:

: Well\, taint was never designed to catch everything. But it's probably : reasonable to restrict method lookups from using tainted data.

So I repent.

Any takers for the patch?

In other words:

$obj->$method... &{$subref}... $subref->...

should die if $method or $subref are tainted.

-- $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 @vanstyn

Jarkko Hietaniemi \jhi@iki\.fi wrote: :More: in Larry's opinion the larger issue of control flow is not :actually larger: there's a decision made based on tainted data\, but :it's a boolean decision\, as opposed to running arbitrary code (which :the method and subroutine indirection allows). : :Also\, an earlier quote: : :: Well\, taint was never designed to catch everything. But it's probably :: reasonable to restrict method lookups from using tainted data. : :> > So I repent. :> :> Any takers for the patch? : :In other words: : : $obj->$method... : &{$subref}... : $subref->... : :should die if $method or $subref are tainted.

I don't think we should try to shoehorn this into 5.8 - it is too big a change for this late stage. For 5.10 I think we should consider introducing several classes of taint checks and allowing a program (at some level or another) to control which classes of checks to enable - I think we can do that without much additional overhead. I think that the existing '-T' should still have substantially the same effect though\, ie it should enable only the class(es) that map to the areas currently checked.

Hugo

p5pRT commented 22 years ago

From @jhi

On Fri\, Apr 12\, 2002 at 06:04:40PM +0100\, Hugo van der Sanden wrote:

Jarkko Hietaniemi \jhi@iki\.fi wrote: :More: in Larry's opinion the larger issue of control flow is not :actually larger: there's a decision made based on tainted data\, but :it's a boolean decision\, as opposed to running arbitrary code (which :the method and subroutine indirection allows). : :Also\, an earlier quote: : :: Well\, taint was never designed to catch everything. But it's probably :: reasonable to restrict method lookups from using tainted data. : :> > So I repent. :> :> Any takers for the patch? : :In other words: : : $obj->$method... : &{$subref}... : $subref->... : :should die if $method or $subref are tainted.

I don't think we should try to shoehorn this into 5.8 - it is too big a change for this late stage. For 5.10 I think we should consider

I would be all too happy to agree\, 5.8 is burning in my pocket\, so to speak. Ccing Larry into the discussion so that I don't have to play a medium.

introducing several classes of taint checks and allowing a program (at some level or another) to control which classes of checks to enable - I think we can do that without much additional overhead. I think that the existing '-T' should still have substantially the same effect though\, ie it should enable only the class(es) that map to the areas currently checked.

Hugo

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

p5pRT commented 14 years ago

From @iabyn

In a still-open ticket from 2002\, it was pointed out that

&$tainted() would not fail under -T\, i.e. external tainted data could affect the internal flow control of a program; in that case make it call any function attacker required. There was then quite a long thread about this\, with Jarkko getting some feedback from Larry\, which seem to come to one of two conclusions:

1) the following three should die under -T:

$obj->$tainted... &{$tainted}... $tainted->...

2) "introducing several classes of taint checks and allowing a program (at some level or another) to control which classes of checks to enable"

(2) sounds complicated. I'm tempted to provide fixes for (1). (Note that the examples in (1) all would have failed anyway if use strict was in place).

-- Any [programming] language that doesn't occasionally surprise the novice will pay for it by continually surprising the expert. -- Larry Wall

p5pRT commented 14 years ago

From @dgl

Hi\,

On Sat\, Mar 20\, 2010 at 04:48:54PM +0000\, Dave Mitchell wrote: [..]

1) the following three should die under -T:
$obj\->$tainted\.\.\.
&\{$tainted\}\.\.\.
$tainted\->\.\.\.

[..]

(2) sounds complicated. I'm tempted to provide fixes for (1). (Note that the examples in (1) all would have failed anyway if use strict was in place).

I think only the middle one wouldn't be allowed under strict..

FOO=UNIVERSAL::isa perl -Mstrict -T -E'my $foo = bless{}; my $x = $ENV{FOO}; $foo->$x'

FOO=UNIVERSAL perl -Mstrict -T -E'my $foo = bless {}; my $x = $ENV{FOO}; $x->isa'

The "$foo->$x" notation is especially scary; like another comment mentions I've seen code where people use it in a dispatcher based on user input.

David

p5pRT commented 14 years ago

From @abigail

On Sat\, Mar 20\, 2010 at 04:48:54PM +0000\, Dave Mitchell wrote:

In a still-open ticket from 2002\, it was pointed out that

&$tainted() would not fail under -T\, i.e. external tainted data could affect the internal flow control of a program; in that case make it call any function attacker required. There was then quite a long thread about this\, with Jarkko getting some feedback from Larry\, which seem to come to one of two conclusions:

1) the following three should die under -T:
$obj\->$tainted\.\.\.
&\{$tainted\}\.\.\.
$tainted\->\.\.\.
2) "introducing several classes of taint checks and allowing a program (at some level or another) to control which classes of checks to enable"

(2) sounds complicated. I'm tempted to provide fixes for (1). (Note that the examples in (1) all would have failed anyway if use strict was in place).

1) is explicitely *excluded* from taint checking; quoting 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:

? Arguments to "print" and "syswrite" are not checked for taintedness.

? Symbolic methods

$obj->$method(@args);

and symbolic sub references

&{$foo}(@args); $foo->(@args);

are not checked for taintedness.

Changing that would not be a bug-fix\, but a policy change.

Abigail

p5pRT commented 14 years ago

From @iabyn

On Tue\, Mar 23\, 2010 at 01:33:52PM +0100\, Abigail wrote:

On Sat\, Mar 20\, 2010 at 04:48:54PM +0000\, Dave Mitchell wrote:
In a still-open ticket from 2002\, it was pointed out that

&$tainted() would not fail under -T\, i.e. external tainted data could affect the internal flow control of a program; in that case make it call any function attacker required. There was then quite a long thread about this\, with Jarkko getting some feedback from Larry\, which seem to come to one of two conclusions:

1) the following three should die under -T:
$obj\->$tainted\.\.\.
&\{$tainted\}\.\.\.
$tainted\->\.\.\.
2) "introducing several classes of taint checks and allowing a program (at some level or another) to control which classes of checks to enable"

(2) sounds complicated. I'm tempted to provide fixes for (1). (Note that the examples in (1) all would have failed anyway if use strict was in place).
1) is explicitely *excluded* from taint checking; quoting 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;:

   ?   Arguments to "print" and "syswrite" are not checked for
       taintedness\.

   ?   Symbolic methods

           $obj\->$method$@&#8203;args$;

       and symbolic sub references

           &\{$foo\}$@&#8203;args$;
           $foo\->$@&#8203;args$;

       are not checked for taintedness\.
Changing that would not be a bug-fix\, but a policy change.

Yeah thanks. After thinking about it some more\, I've downgrade the ticket to notabug/wishlist\, and leave it for someone else to add add a 'use taint "subs"' style facility if they so desire.

I;ve also noticed you can do 'goto $tainted_label' and there's probably a whole world of such stuff out there.

-- Wesley Crusher gets beaten up by his classmates for being a smarmy git\, and consequently has a go at making some friends of his own age for a change. -- Things That Never Happen in "Star Trek" #18

p5pRT commented 14 years ago

From @iabyn

I've downgraded this ticket to notabug/wishlist on the grounds that its not a bug\, and that any changes to make taint complain about internal stuff should be done by a new pragma if at all.

p5pRT commented 14 years ago

From @abigail

On Wed\, Mar 24\, 2010 at 10:54:41PM +0000\, Dave Mitchell wrote:

On Tue\, Mar 23\, 2010 at 01:33:52PM +0100\, Abigail wrote:

Changing that would not be a bug-fix\, but a policy change.

Yeah thanks. After thinking about it some more\, I've downgrade the ticket to notabug/wishlist\, and leave it for someone else to add add a 'use taint "subs"' style facility if they so desire.

I;ve also noticed you can do 'goto $tainted_label' and there's probably a whole world of such stuff out there.

Yeah\, and we can do

while ($i ++ != $tainted_value) { ... }

as well.

I never saw that as a problem. I always considered tainting checks whether I'm using a tainted value to change something outside of the system.

Abigail

p5pRT commented 14 years ago

@chorny - Status changed from 'open' to 'stalled'

Perl / perl5