Re: Tainting problem on Strawberry perl

I don't see anything wrong in the dumps, and you didn't point out anything specific (Update: OP has since been updated to include expected+actual values), but these two tips should help.

Don't pass globals to functions that may modify them. (I consider anything in perlvar unsafe.)

$wdir = File::Spec->catfile($cdir, $1);    # Bad
$wdir = File::Spec->catfile($cdir, "$1");  # Good, $1 is copied.
[download]

Avoid using $1, $2, anywhere except immediately after where they are set, and only to copy them. This may not be necessary, but it is definitely safe, worry-free and better than using variables with meaningless names.

By the way, your untainting is rather arbitrary (it ignores anything starting with the first \x0A) and defies the whole idea of tainting (it allows everything else including \x00).

Comment on Re: Tainting problem on Strawberry perl Select or Download Code

Replies are listed 'Best First'.
Re^2: Tainting problem on Strawberry perl by EvanCarroll (Chaplain) on Feb 28, 2008 at 22:43 UTC
Just to clarify this isn't my code, this is the code from Template::Provider, I'm trying to get my Catalyst stuff to work on IIS. Onward, your suggestion is sound and `$wdir = File::Spec->catfile($cdir, "$1");` even works, but the question remains, how is it broken? What makes my Strawberry 5.10 different from most other peoples is that I keep the components in it fairly up to date. Which component could have broken this? File::Spec maybe? and should this bug be fixed in Tempalte::Provider or elsewhere (which is what I lean towards). Evan Carroll www.EvanCarroll.com	[reply] [d/l]
Re^3: Tainting problem on Strawberry perl by ikegami (Patriarch) on Feb 28, 2008 at 23:04 UTC
I don't know why, sorry, but I can confirm it's probably a bug in your build. It doesn't affect my ActivePerl builds. I simplified your code to `use File::Spec::Win32; my $wdir = 'C:/strawberry'; $wdir =~ s[:][]g; $wdir =~ /(.*)/; print File::Spec::Win32->catfile('C:/cache', $1), "\n";` [download] and I get (ActivePerl 5.8.8 & 5.10.0) `C:\cache\C\strawberry` [download] You presumably get `C:\cache\C:` [download]	[reply] [d/l] [select]
Re^4: Tainting problem on Strawberry perl by EvanCarroll (Chaplain) on Feb 28, 2008 at 23:09 UTC
Wow you found it, in that you made a test, I guess it has nothing to do with a being tainted, I'll rename node. That is correct with your code, I get what you've assumed I would get. Evan Carroll www.EvanCarroll.com	[reply]
Re^5: Tainting problem on Strawberry perl by ikegami (Patriarch) on Feb 28, 2008 at 23:33 UTC
Re^3: Tainting problem on Strawberry perl by syphilis (Archbishop) on Feb 28, 2008 at 23:42 UTC
Which component could have broken this? File::Spec maybe? Looks likely. Using the test case that ikegami provided, I find that I get his (correct) output with File::Spec::Win32-3.2501. But when I update to PathTools-3.2701, I get your (broken) output. Cheers, Rob	[reply]
Re^4: Tainting problem on Strawberry perl by ikegami (Patriarch) on Feb 28, 2008 at 23:52 UTC
Bingo! It is as I suspected in my initial post. I was confused in my latter posts because I was looking at the wrong version. Changes to `$1` results in changes to `$_[1]`, since `$_[1]` is aliased to `$1`. I was looking at the version in perl-5.10.0, not the one from PathTools-3.2701. The one in perl-5.10.0 copies `@_` into local variables, so it works fine. The one in Pathtools-3.2701 works with `@_` directly, so it picks up the change to `$1`. That technically makes the bug PathTools's, since it should localize the globlals it modifies (like `$1`). However, it's rarely done (especially for `$1`, `$!` and `$@`) and it's actually rather hard to do when working with `@_` directly and package variables (like `$1`), so it's best not to pass globals to functions and to follow the tips I already posted. Update: Here's some code the illustrate the issue. `sub f_bad { 'b' =~ /(.)/; return $_[0]; } sub f_good1 { my ($var) = @_; 'b' =~ /(.)/; return $var; } sub f_good2 { { local $1; 'b' =~ /(.)/; } return $_[0]; } 'a' =~ /(.)/; print f_bad($1), "\n"; # b 'a' =~ /(.)/; print f_good1($1), "\n"; # a 'a' =~ /(.)/; print f_good2($1), "\n"; # a` [download] Unforunately, `sub { local $1; 'b' =~ /(.*)/; return $_[0]; }` doesn't work since `local` doesn't create a new variable.	[reply] [d/l] [select]