develooper Front page | perl.perl5.porters | Postings from February 2000

Re: tainting oddities (some doc, some code)

Thread Previous
From:
Gurusamy Sarathy
Date:
February 27, 2000 08:56
Subject:
Re: tainting oddities (some doc, some code)
Message ID:
200002271658.IAA28106@maul.activestate.com
On Sat, 26 Feb 2000 08:46:25 MST, Tom Christiansen wrote:
>	    /*
>	     * The external globbing program may use things we can't control,
>	     * so for security reasons we must assume the worst.
>	     */
>	    TAINT;
>	    taint_proper(PL_no_security, "glob");
[...]
>And perlsec has this:
>
>    @files = <*.c>;             # Always insecure (uses csh)
>    @files = glob('*.c');       # Always insecure (uses csh)
>
>At the very least, the comments are misleading.  glob() no longer
>uses an external program, so the "Always" part is wrong.  Likewise,
>the pp_sys.c comment is problematic: what if you're not using
>an external globbing?  
>
>The answer is that at some level, it comes down to readdir, which
>is now tainted.
[...]
>So, in theory this should apply to globbing, too.  And in fact, it
>does, as you notice if you you look at Glob.xs.  It's just not for
>the reasons given in the previously cited comments, which should be
>altered.
>
>It also occurs to me that readdir() doesn't allow untainting of the
>handle.  I'd kinda expect that since you can untaint a filehandle,
>you should be able to do likewise on a directory handle.

These are fixed by the patches below.

>Other tainting oddities: 
>
> *  msgrcv() and msgget() (and probably any the other revelant 
>    SysV IPC functions), even though recv() does.  This seems 
>    incorrect.
>
> *  While the pw_gecos is tainted because it's in theory mungible,
>    the pw_shell is not.  However, on some systems, you are allowed
>    to change your shell to something not in /etc/shells.

Any second opinions on these?


Sarathy
gsar@ActiveState.com
-----------------------------------8<-----------------------------------
Change 5285 by gsar@auger on 2000/02/27 16:20:31

	make readdir() respect IOf_UNTAINT flag (allows untainting of directory
	handles with: C<use IO::Handle; opendir D, $dir or die; D->untaint;>

Affected files ...

... //depot/perl/pp_sys.c#149 edit

Differences ...

==== //depot/perl/pp_sys.c#149 (text) ====
Index: perl/pp_sys.c
--- perl/pp_sys.c.~1~	Sun Feb 27 08:53:09 2000
+++ perl/pp_sys.c	Sun Feb 27 08:53:09 2000
@@ -3462,7 +3462,8 @@
 	    sv = newSVpv(dp->d_name, 0);
 #endif
 #ifndef INCOMPLETE_TAINTS
-  	    SvTAINTED_on(sv);
+	    if (!(IoFLAGS(io) & IOf_UNTAINT))
+		SvTAINTED_on(sv);
 #endif
 	    XPUSHs(sv_2mortal(sv));
 	}
@@ -3476,7 +3477,8 @@
 	sv = newSVpv(dp->d_name, 0);
 #endif
 #ifndef INCOMPLETE_TAINTS
-	SvTAINTED_on(sv);
+	if (!(IoFLAGS(io) & IOf_UNTAINT))
+	    SvTAINTED_on(sv);
 #endif
 	XPUSHs(sv_2mortal(sv));
     }
End of Patch.

Change 5286 by gsar@auger on 2000/02/27 16:52:54

	remove outdated info about csh and glob(); glob() need not fail
	when tainting anymore if using internal globbing

Affected files ...

... //depot/perl/pod/perldelta.pod#163 edit
... //depot/perl/pod/perlfaq5.pod#13 edit
... //depot/perl/pod/perlfunc.pod#154 edit
... //depot/perl/pod/perlop.pod#60 edit
... //depot/perl/pod/perlsec.pod#12 edit
... //depot/perl/pod/perltodo.pod#12 edit
... //depot/perl/pp_sys.c#150 edit

Differences ...

==== //depot/perl/pod/perldelta.pod#163 (text) ====
Index: perl/pod/perldelta.pod
--- perl/pod/perldelta.pod.~1~	Sun Feb 27 08:53:19 2000
+++ perl/pod/perldelta.pod	Sun Feb 27 08:53:19 2000
@@ -1280,7 +1280,7 @@
 token; if not, the DATA filehandle will be left open in binary mode.
 Earlier versions always opened the DATA filehandle in text mode.
 
-The glob() operator is implemented via the L<File::Glob> extension,
+The glob() operator is implemented via the C<File::Glob> extension,
 which supports glob syntax of the C shell.  This increases the flexibility
 of the glob() operator, but there may be compatibility issues for
 programs that relied on the older globbing syntax.  If you want to

==== //depot/perl/pod/perlfaq5.pod#13 (text) ====
Index: perl/pod/perlfaq5.pod
--- perl/pod/perlfaq5.pod.~1~	Sun Feb 27 08:53:19 2000
+++ perl/pod/perlfaq5.pod	Sun Feb 27 08:53:19 2000
@@ -559,14 +559,15 @@
 =head2 Why do I sometimes get an "Argument list too long" when I use E<lt>*E<gt>?
 
 The C<E<lt>E<gt>> operator performs a globbing operation (see above).
-By default glob() forks csh(1) to do the actual glob expansion, but
+In Perl versions earlier than v5.6.0, the internal glob() operator forks
+csh(1) to do the actual glob expansion, but
 csh can't handle more than 127 items and so gives the error message
 C<Argument list too long>.  People who installed tcsh as csh won't
 have this problem, but their users may be surprised by it.
 
-To get around this, either do the glob yourself with readdir() and
-patterns, or use a module like Glob::KGlob, one that doesn't use the
-shell to do globbing.  This is expected to be fixed soon.
+To get around this, either upgrade to Perl v5.6.0 or later, do the glob
+yourself with readdir() and patterns, or use a module like Glob::KGlob,
+one that doesn't use the shell to do globbing.
 
 =head2 Is there a leak/bug in glob()?
 

==== //depot/perl/pod/perlfunc.pod#154 (text) ====
Index: perl/pod/perlfunc.pod
--- perl/pod/perlfunc.pod.~1~	Sun Feb 27 08:53:19 2000
+++ perl/pod/perlfunc.pod	Sun Feb 27 08:53:19 2000
@@ -1937,6 +1937,9 @@
 If EXPR is omitted, C<$_> is used.  The C<E<lt>*.cE<gt>> operator is
 discussed in more detail in L<perlop/"I/O Operators">.
 
+Beginning with v5.6.0, this operator is implemented using the standard
+C<File::Glob> extension.  See L<File::Glob> for details.
+
 =item gmtime EXPR
 
 Converts a time as returned by the time function to a 9-element list

==== //depot/perl/pod/perlop.pod#60 (text) ====
Index: perl/pod/perlop.pod
--- perl/pod/perlop.pod.~1~	Sun Feb 27 08:53:19 2000
+++ perl/pod/perlop.pod	Sun Feb 27 08:53:19 2000
@@ -1696,7 +1696,7 @@
 	chmod 0644, $_;
     }
 
-is equivalent to
+is roughly equivalent to:
 
     open(FOO, "echo *.c | tr -s ' \t\r\f' '\\012\\012\\012\\012'|");
     while (<FOO>) {
@@ -1704,20 +1704,11 @@
 	chmod 0644, $_;
     }
 
-In fact, it's currently implemented that way, but this is expected
-to be made completely internal in the near future.  (Which means
-it will not work on filenames with spaces in them unless you have
-csh(1) on your machine.)  Of course, the shortest way to do the
-above is:
+except that the globbing is actually done internally using the standard
+C<File::Glob> extension.  Of course, the shortest way to do the above is:
 
     chmod 0644, <*.c>;
 
-Because globbing currently invokes a shell, it's often faster to
-call readdir() yourself and do your own grep() on the filenames.
-Furthermore, due to its current implementation of using a shell,
-the glob() routine may get "Arg list too long" errors (unless you've
-installed tcsh(1L) as F</bin/csh> or hacked your F<config.sh>).
-
 A (file)glob evaluates its (embedded) argument only when it is
 starting a new list.  All values must be read before it will start
 over.  In list context, this isn't important because you automatically

==== //depot/perl/pod/perlsec.pod#12 (text) ====
Index: perl/pod/perlsec.pod
--- perl/pod/perlsec.pod.~1~	Sun Feb 27 08:53:19 2000
+++ perl/pod/perlsec.pod	Sun Feb 27 08:53:19 2000
@@ -84,8 +84,8 @@
     exec "echo", $arg;		# Secure (doesn't use the shell)
     exec "sh", '-c', $arg;	# Considered secure, alas!
 
-    @files = <*.c>;		# Always insecure (uses csh)
-    @files = glob('*.c');	# Always insecure (uses csh)
+    @files = <*.c>;		# insecure (uses readdir() or similar)
+    @files = glob('*.c');	# insecure (uses readdir() or similar)
 
 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

==== //depot/perl/pod/perltodo.pod#12 (text) ====
Index: perl/pod/perltodo.pod
--- perl/pod/perltodo.pod.~1~	Sun Feb 27 08:53:19 2000
+++ perl/pod/perltodo.pod	Sun Feb 27 08:53:19 2000
@@ -147,14 +147,6 @@
 
 =back
 
-=head2 Built-in globbing
-
-Currently the C<E<lt>*.cE<gt>> syntax calls the c shell.  This causes
-problems on sites without csh, systems where fork() is expensive, and
-setuid environments.  Decide between Glob::BSD and File::KGlob, move
-it into the core, and make Perl use it for globbing.  Ben Holzman and
-Tye McQueen have claimed the pumpkin for this.
-
 =head1 Perl Internals
 
 =head2 magic_setisa
@@ -593,20 +585,12 @@
 
 =head1 Win32 Stuff
 
-=head2 Get PERL_OBJECT building under gcc
-
-B<Part done>, according to Sarathy.  It builds under egcs on win32,
-but doesn't run for occult reasons.  If anyone knows the right
-breed of chicken to sacrifice, please speak up.
-
 =head2 Rename new headers to be consistent with the rest
 
 =head2 Sort out the spawnvp() mess
 
 =head2 Work out DLL versioning
 
-=head2 Get PERL_OBJECT building on non-win32
-
 =head2 Style-check
 
 =head1 Would be nice to have
@@ -853,14 +837,10 @@
 
 =head2 Filenames
 
-Make filenames in the distribution and in the standard module set
+Keep filenames in the distribution and in the standard module set
 be 8.3 friendly where feasible.  Good luck changing the standard
-modules, though.  B<Done>.
-
-=head2 Proper tied array support
+modules, though.
 
-This was B<done> in 5.005 by Nick Ing-Simmons.
-
 =head2 Foreign lines
 
 Perl should be more generous in accepting foreign line terminations.
@@ -873,49 +853,18 @@
 
     CPP-space:	  stop malloc()/free() pollution unless asked
 
-=head2 Explain tool
-
-Given a piece of Perl code, say what it does.  B::Deparse is doing
-this.  B<Done>.
-
 =head2 ISA.pm
 
 Rename and alter ISA.pm.  B<Done>.  It is now base.pm.
 
-=head2 Automate maintenance of most PERL_OBJECT code
-
-B<Done>, says Sarathy.
-
-=head2 -iprefix.
-
-Added in 5.004_70.  B<Done>
-
 =head2 gettimeofday
 
 See Time::HiRes.
 
-=head2 reference to compiled regexp
-
-B<done>  This is the qr// support in 5.005.
-
-=head2 eval qw() at compile time
-
-qw() is presently compiled as a call to split.  This means the split
-happens at runtime.  Change this so qw() is compiled as a real list
-assignment.  This also avoids surprises like:
-
-    $a = () = qw(What will $a hold?);
-
-B<Done>.  Tom Hughes submitted a patch that went into 5.005_55.
-
 =head2 autocroak?
 
-B<Done>.  This is the Fatal.pm module, so any builtin that that does
+This is the Fatal.pm module, so any builtin that that does
 not return success automatically die()s.  If you're feeling brave, tie
 this in with the unified exceptions scheme.
 
-=head2 Status variable
-
-$^C to track compiler/checker status.  B<Done> in 5.005_54.
-
 =cut

==== //depot/perl/pp_sys.c#150 (text) ====
Index: perl/pp_sys.c
--- perl/pp_sys.c.~1~	Sun Feb 27 08:53:19 2000
+++ perl/pp_sys.c	Sun Feb 27 08:53:19 2000
@@ -359,6 +359,9 @@
     ENTER;
 
 #ifndef VMS
+    /* If we're not using an external glob, just let readdir() tainting
+     * do its thing.  Otherwise, engage paranoia mode. */
+#if defined(PERL_EXTERNAL_GLOB)
     if (PL_tainting) {
 	/*
 	 * The external globbing program may use things we can't control,
@@ -367,6 +370,7 @@
 	TAINT;
 	taint_proper(PL_no_security, "glob");
     }
+#endif /* PERL_EXTERNAL_GLOB */
 #endif /* !VMS */
 
     SAVESPTR(PL_last_in_gv);	/* We don't want this to be permanent. */
End of Patch.

Thread Previous


nntp.perl.org: Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at ask@perl.org | Group listing | About