[Petal] More on taint issues with Petal 1.10_xx

[William McKee]

On Thu, Oct 16, 2003 at 04:50:24PM +0100, Jean-Michel Hiver wrote:
> I must admit that I have disabled the Taint code from Petal 1.10_06
> onwards because:
> 
> 1/ It's no use to me
> 2/ I have been focusing on other issues

Fair enough.


> That being said, if you can come up with a patch that'll make things
> work for you I'll be very happy to integrate it into the coming Petal
> 1.10.

I'd be glad if you'd incorporate the attached diff. Most of my pages are
working. The splitpath error in CGI::Carp (which initiated the
discussion that led to your pulling the TAINT code) is only coming up on
more complicated pages. Unfortunately, I haven't been able to reproduce
it in my test template :(. Kurt, could you send me your template that
produces the splitpath error? I'd be interested in comparing the two to
see if I can find any similarities.

At any rate, with this patch, if someone is having this problem on a
particular page, all they need to do is set $Petal::TAINT=0 before
processing the page. Even if they are using -T like I am, they will
receive no errors if using MKDoc::XML as the parser since it effectively
clears the taintedness of the template.  I know there is a way to set
the parser for HTML::TB but is there a similar way to change it back to
MKDoc::XML? Curiously, I also get the splitpath error with
Petal::Parser::HTB. I had originally thought the template would run fine
in HTB but not MKDoc::XML.

I've tried debugging with the Perl debugger but get segmentation faults
when Safe tries to eval. This would make you think the code in error is
within the code being evaluated. However there are no calls to die,
warn, confess or carp and, after fixing the attr bug, the generated code
compiles without error. Of course, I'm also using several additional
objects in the hashref that I'm passing into the process sub. However,
changing that to undef or an empty hashref continued to produce the
splitpath error.

Engine hackers: Is there a way to run the coderef that the process() sub
generates outside of Petal? It seems that would be an interesting test.

Oh, there are a couple of new die statements in the process sub. I added
these because I am getting back empty $coderef's on the first run when
using Safe (but only on documents that cause the splitpath error;
documents that work with Safe always work). Subsequent runs all produce
the splitpath error. Petal was giving the following meaningless error
when $coderef was null:

[PETAL ERROR] Can't use string ("") as a subroutine ref while "strict
refs" in use at /usr/local/share/perl/5.6.1/Petal.pm line 351.

Finally, I have submitted the test script that generates the splitpath
error that I posted to the list yesterday to Lincoln Stein. I'll let you
know if I hear anything back from him. If only CGI::Carp would play nice
with Safe we wouldn't have had to go through all this mess. BTW, has
anyone tried running this script? (Yeah, I know it's the weekend, but I'm
on a mission!)

Thanks,
William

-- 
Knowmad Services Inc.
http://www.knowmad.com
-------------- next part --------------
--- ../src/Petal-1.10_08/lib/Petal.pm	Thu Oct 16 11:55:23 2003
+++ /usr/local/share/perl/5.6.1/Petal.pm	Sat Oct 18 17:45:21 2003
@@ -346,6 +346,8 @@
 	else                            { $hash = new Petal::Hash (@_)         }
 	
 	my $coderef = $self->_code_memory_cached;
+	die "\$coderef is undefined\n\n" unless $coderef;
+	die "\$hash is undefined\n\n" unless $hash;
 	$res = $coderef->($hash);
 	
 	$Petal::ENCODE_CHARSET and do {
@@ -594,9 +596,25 @@
 	my $code_perl = $self->_code_disk_cached;
 	my $VAR1 = undef;
 	
-	eval "$code_perl";
-	confess ($@ . "\n" . $self->_code_with_line_numbers) if $@;
-	$code = $VAR1;
+	if ($TAINT)
+	{
+		# important line, don't remove
+		($code_perl) = $code_perl =~ m/^(.+)$/s;
+		die "\$code_perl is empty after untainting!" unless defined $code_perl && $code_perl;
+		my $cpt = Safe->new ("Petal::CPT");
+		$cpt->permit ('entereval');
+		$cpt->permit ('leaveeval');
+		$cpt->permit ('require');
+		$code = $cpt->reval($code_perl);
+		confess ("Error in reval:\n" . $@ . "\n" . $self->_code_with_line_numbers) if $@;
+		warn "\$code is empty after reval.\n" . Dumper($code, $Petal::CPT::VAR1, length($code_perl)) unless $code;
+	}
+	else
+	{
+	    eval "$code_perl";
+	    confess ($@ . "\n" . $self->_code_with_line_numbers) if $@;
+	    $code = $VAR1;
+	}
 	
 	Petal::Cache::Memory->set ($self->_file_path_with_macro, $code) if (defined $MEMORY_CACHE and $MEMORY_CACHE);	
     }
@@ -605,33 +623,7 @@
 }
 
 
-=cut
-
-#($code_perl) = $code_perl =~ m/^(.+)$/s;
-#if ($TAINT)
-#{
-#   # important line, don't remove
-#	    ($code_perl) = $code_perl =~ m/^(.+)$/s;
-#	    my $cpt = Safe->new ("Petal::CPT");
-#	    $cpt->permit ('entereval');
-#	    $cpt->permit ('leaveeval');
-#	    $cpt->permit ('require');
-#	    $cpt->reval ($code_perl);
-#	    confess ($@ . "\n" . $self->_code_with_line_numbers) if $@;
-#	    
-#	    # remove silly warning '"Petal::CPT::VAR1" used only once'
-#	    $Petal::CPT::VAR1 if (0);
-#	    $code = $Petal::CPT::VAR1;
-#	}
-#	else
-#	{
-#	    eval "$code_perl";
-#	    confess ($@ . "\n" . $self->_code_with_line_numbers) if $@;
-#	    $code = $VAR1;
-#	}
 
-=cut
-	
 
 
 # $self->_code_cache;