develooper Front page | perl.perl5.porters | Postings from October 2003

[PATCH] Re: [perl #24291] Taint checking against the wrong environment

Thread Previous | Thread Next
From:
Rafael Garcia-Suarez
Date:
October 27, 2003 02:53
Subject:
[PATCH] Re: [perl #24291] Taint checking against the wrong environment
Message ID:
20031027115225.7c21a92c.rgarciasuarez@free.fr
I proposed :
> 
> Other proposal : in TAINT_ENV, the routine that checks for a tainted
> environment, croak() if %ENV hasn't environment-magic, or if the hash
> slot of the *ENV glob is empty. This modifies only the behaviour of perl
> with -T.

With the patch below, I have these results :

    $ ./perl -Twle '%a=(a=>4, b=>5); *ENV=\%a; system("echo unsafe")'
    %ENV is aliased to another variable while running with -T switch at -e line 1.

    $ ./perl -Twle '%a=(a=>4, b=>5); *ENV=*a; system("echo unsafe")'
    %ENV is aliased to %a while running with -T switch at -e line 1.

i.e. when doing a TAINT_ENV check, immediately croak if %ENV is no
longer referring to the environment. This early failure approach
solves the security problem.

The behaviour of perl is not changed when not in -T or -t mode : i.e.
aliasing the *ENV glob works like regular glob aliasing.

If no objections or improvement proposals, I'll commit this.


Index: pod/perldiag.pod
===================================================================
--- pod/perldiag.pod	(revision 2730)
+++ pod/perldiag.pod	(working copy)
@@ -1387,6 +1387,12 @@
 (F) While under the C<use filetest> pragma, switching the real and
 effective uids or gids failed.
 
+=item %ENV is aliased to %s
+
+(F) You're running under taint mode, and the C<%ENV> variable has been
+aliased to another hash, so it doesn't reflect anymore the state of the
+program's environment. This is potentially insecure.
+
 =item Error converting file specification %s
 
 (F) An error peculiar to VMS.  Because Perl may have to deal with file
Index: t/op/taint.t
===================================================================
--- t/op/taint.t	(revision 2744)
+++ t/op/taint.t	(working copy)
@@ -984,13 +984,12 @@
 }
 
 {
-    # test with a non-magical %ENV (and non-magical %ENV elements)
-    our %nonmagicalenv = ( PATH => $TAINT );
+    # [perl #24291] this used to dump core
+    our %nonmagicalenv = ( PATH => "util" );
     local *ENV = \%nonmagicalenv;
     eval { system("lskdfj"); };
-    test 207, $@ =~ /Insecure \$ENV{PATH} while running with -T switch/;
-    # [perl #24291] this used to dump core
-    %nonmagicalenv = ( PATH => "util" );
+    test 207, $@ =~ /^%ENV is aliased to another variable while running with -T switch/;
+    local *ENV = *nonmagicalenv;
     eval { system("lskdfj"); };
-    test 208, 1;
+    test 208, $@ =~ /^%ENV is aliased to %nonmagicalenv while running with -T switch/;
 }
Index: taint.c
===================================================================
--- taint.c	(revision 2744)
+++ taint.c	(working copy)
@@ -80,9 +80,25 @@
 	NULL
     };
 
-    /* Don't bother if there's no %ENV hash */
-    if (!PL_envgv || !GvHV(PL_envgv))
+    /* Don't bother if there's no *ENV glob */
+    if (!PL_envgv)
 	return;
+    /* If there's no %ENV hash of if it's not magical, croak, because
+     * it probably doesn't reflect the actual environment */
+    if (!GvHV(PL_envgv) || !(SvRMAGICAL(GvHV(PL_envgv))
+	    && mg_find((SV*)GvHV(PL_envgv), PERL_MAGIC_env))) {
+	bool was_tainted = PL_tainted;
+	char *name = GvENAME(PL_envgv);
+	PL_tainted = TRUE;
+	if (strEQ(name,"ENV"))
+	    /* hash alias */
+	    taint_proper("%%ENV is aliased to %s%s", "another variable");
+	else
+	    /* glob alias: report it in the error message */
+	    taint_proper("%%ENV is aliased to %%%s%s", name);
+	/* this statement is reached under -t or -U */
+	PL_tainted = was_tainted;
+    }
 
 #ifdef VMS
     {
@@ -99,9 +115,7 @@
 	    TAINT;
 	    taint_proper("Insecure %s%s", "$ENV{DCL$PATH}");
 	}
-	if (SvMAGICAL(*svp)
-		&& (mg = mg_find(*svp, PERL_MAGIC_envelem))
-		&& MgTAINTEDDIR(mg)) {
+	if ((mg = mg_find(*svp, PERL_MAGIC_envelem)) && MgTAINTEDDIR(mg)) {
 	    TAINT;
 	    taint_proper("Insecure directory in %s%s", "$ENV{DCL$PATH}");
 	}
@@ -116,9 +130,7 @@
 	    TAINT;
 	    taint_proper("Insecure %s%s", "$ENV{PATH}");
 	}
-	if (SvMAGICAL(*svp)
-		&& (mg = mg_find(*svp, PERL_MAGIC_envelem))
-		&& MgTAINTEDDIR(mg)) {
+	if ((mg = mg_find(*svp, PERL_MAGIC_envelem)) && MgTAINTEDDIR(mg)) {
 	    TAINT;
 	    taint_proper("Insecure directory in %s%s", "$ENV{PATH}");
 	}

Thread Previous | Thread Next


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