develooper Front page | perl.perl5.porters | Postings from December 2008

Re: taint copying (was Re: 5.8.9/taint/NYTProf regression)

Thread Previous | Thread Next
From:
Rick Delaney
Date:
December 1, 2008 20:52
Subject:
Re: taint copying (was Re: 5.8.9/taint/NYTProf regression)
Message ID:
20081202045202.GA26304@bort.ca
On Nov 30 2008, Nicholas Clark wrote:
> On Sun, Nov 30, 2008 at 01:54:43PM +0000, Nicholas Clark wrote:
> > On Sun, Nov 30, 2008 at 01:09:37PM +0000, Nicholas Clark wrote:
> > 
> > > I don't fully understand what is going on. A lot of SVs with package names
> > > seem to end up tainted at times during compiling. However, in 5.8.8 (and
> > 
> > Taint gets added at the sv_catpvn of "::" here in Perl_yylex():
> > 
> > 		    else {
> > 			sv_setsv(PL_subname,PL_curstname);
> > 			sv_catpvn(PL_subname,"::",2);
> > 			sv_catpvn(PL_subname,tmpbuf,len);
> > 		    }
> > 
> > So I'm starting to wonder if the code in Perl_leave_scope() that sets
> > PL_localizing = 2 is a hack to remove tainting that probably never should have
> > been there.
> 
> I'm still suspicious that the tainting should never have been there.

I don't understand it all either, especially how PL_subname gets tainted.
The problem is once it gets tainted, it stays tainted since restoring
its value--after localizing via save_item() in Perl_start_subparse()
--doesn't affect the magic it already has.

I think what the old code was doing was making sure that the taintedness
of the saved-away value was restored to localized values at leave_scope
time.  The current code doesn't remove taintedness when restoring an
untainted value to a tainted container.  Restoring taint directly like
below fixes things.  Maybe the change should go right into
Perl_sv_replace() or could be otherwise improved but it works.

diff -pruNb perl-5.8.x/scope.c perl-5.8.9-RC1/scope.c
--- perl-5.8.x/scope.c	2008-10-29 04:23:15.000000000 -0400
+++ perl-5.8.9-RC1/scope.c	2008-12-01 23:27:07.000000000 -0500
@@ -621,12 +621,17 @@ Perl_leave_scope(pTHX_ I32 base)
 	const int type = SSPOPINT;
 	switch (type) {
 	case SAVEt_ITEM:			/* normal string */
+	    {
+		bool was_tainted = 0;
 	    value = (SV*)SSPOPPTR;
 	    sv = (SV*)SSPOPPTR;
+		was_tainted = SvTAINTED(value);
 	    sv_replace(sv,value);
+		was_tainted ? SvTAINTED_on(sv) : SvTAINTED_off(sv);
 	    PL_localizing = 2;
 	    SvSETMAGIC(sv);
 	    PL_localizing = 0;
+	    }
 	    break;
 	case SAVEt_SV:				/* scalar reference */
 	    value = (SV*)SSPOPPTR;

-- 
Rick Delaney
rick@bort.ca

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