Bug #96291 for Inline: t/08taint.t fails on perl 5.20.0

Confirmation from #perl on irc.perl.org - it's a deliberate change in perl 5.20.0. A quick fix would be either to explicitly set $ENV{PATH} to '/bin:/usr/bin', or skip the test for 5.20.0. The rationale is that taint mode is rarely used anymore. The following suggestion was made: <@mst> maybe the best way to fix the test would be to try /bin, /usr/bin, /usr/local/bin <@mst> and see if the necessary binaries are there <@mst> and if yes, you can run the test <@mst> and if no, skip all <@mst> but still run the test in cases where we can do I hoped there would be a sensible value available in %Config, but there isn't.

Hi Rob, Per the discussion with mst on #perl (ex pumpkin holder), I propose (and will do if you haven't already) that at the top of 08taint.t: 1. Check for existence of $ENV{PATH} 2. If not, set to '/bin:/usr/bin' 3. Test in $ENV{PATH} for 'make' and $Config{cc} 4. If found, continue; if not, skip (since there's nothing else reasonable to do, and I prefer not to make people force install) Do you approve of this strategy? On the systems you tested on, did Configure find "truly secure setuid scripts"? Mine said no - I predict that's why it zeroes the path. Cheers, Ed Show quoted text

On Fri Jun 06 20:50:00 2014, ETJ wrote: Show quoted text
I'd like to report that I'm having this same problem on EVERY gentoo system I've tried, each of varying architecture and updated-ness. It worked fine on Linux Mint 15 and 16.

If no-one else wants to, I'll do both the test-possible-skipping and a doc update? It would probably be a candidate for a fast new release.

On Mon Jun 09 01:00:03 2014, NERDVANA wrote: Show quoted text
Reason was the logic in Inline.pm untainting PATH disallows any directories writable by that user - for root, that's all of them! Change proposed is visible on github.

The failure is because on that test system, input PATH: /srv/smoker/bin:/usr/lib/ccache:/srv/smoker/perl5/perlbrew/bin:/srv/smoker/perl5/perlbrew/perls/perl-5.20.0/bin:/usr/local/bin:/usr/bin:/bin:/usr/local/games:/usr/games is being stripped down to this: /srv/smoker/bin:/usr/lib/ccache:/srv/smoker/perl5/perlbrew/bin:/srv/smoker/perl5/perlbrew/perls/perl-5.20.0/bin:/usr/bin:/usr/games These were removed: /usr/local/bin /bin /usr/local/games The untainting code, on non-Windows (this system is Linux) removes directories from PATH if they are NOT: absolute, directories, and neither group- nor world- writable. The "problem" here is that the relevant system has configured /bin to be either group- or world-writable. It therefore gets removed, so chmod (which typically lives in /bin) is unavailable. The issue we face here is that tainting is designed to deal with two different situations: CGIs, and suid scripts on multi-user systems. A. For CGIs, there is no point in stripping the PATH, because the entire content of the system is under the control of the admin, and the only threat is web-user input. B. For suid scripts on multi-user systems, there IS a point to stripping the PATH, because a malicious user could provide a PATH where e.g. a chmod command under their control would be found before the "real" one. However, the "correct" value to set PATH to is probably not by stripping some values out, but by setting it to a known value, eg "/bin:/usr/bin:/usr/local/bin". This might be problematic because that will not always be the correct value for a given system, and would therefore need to be configured on installation, which is not a road Inline has yet needed to go down. I believe there are two decent ways forward here: 1. document that Inline does not build when used in taint mode (although it can still safely run code) and make it be a fatal error to try to do so; 2. update the PATH-untainting code to being nearly what it was before I changed it, but instead of "-w $_ || -W $_", which I believe was a mistake, since it means "writable by either effective or real uid", make it "-W $_" - "writable by real uid". I advocate method 2, since it deals effectively with situations A and B (including the real threat in B), and will almost certainly pass on the system that failed the test. The following patch implements it, and all the tests still pass on my Linux system: diff --git a/Inline.pm b/Inline.pm index 32868a3..5fced1c 100644 --- a/Inline.pm +++ b/Inline.pm @@ -1075,7 +1075,7 @@ sub env_untaint { join ';', grep {not /^\./ and -d $_ } split /;/, $ENV{PATH} : - join ':', grep {/^\// and -d $_ and not ((stat($_))[2] & 0022) + join ':', grep {/^\// and -d $_ and not -W $_ } split /:/, $ENV{PATH}; map {($_) = /(.*)/} @INC;

On further further reflection, the previous logic is bound to give false positives when running as root, which means installing as root using CPAN (a reasonable thing to do) will fail tests, which is where we came in. Instead, this patch (replacing previous two) actually tests $< != $>, which revealed a couple of quirks, hence a couple of extra changes: diff --git a/C/t/08taint.t b/C/t/08taint.t index 9effb6f..357b551 100644 --- a/C/t/08taint.t +++ b/C/t/08taint.t @@ -21,13 +21,15 @@ BEGIN { use warnings; use strict; use Test::More tests => 10; - use Test::Warn; # Suppress "Set up gcc environment ..." warning. # (Affects ActivePerl only.) $ENV{ACTIVEPERL_CONFIG_SILENT} = 1; +# deal with running as root - actually simulate running as setuid program +$< = 1; # ignore failure + my $w1 = 'Blindly untainting tainted fields in %ENV'; my $w2 = 'Blindly untainting Inline configuration file information'; my $w3 = 'Blindly untainting tainted fields in Inline object'; diff --git a/Inline.pm b/Inline.pm index 32868a3..83f7035 100644 --- a/Inline.pm +++ b/Inline.pm @@ -846,6 +846,8 @@ sub create_config_file { next; } next if $mod =~ /^(MakeMaker|denter|messages)$/; + # @INC is made safe by -T disallowing PERL5LIB et al + ($mod) = $mod =~ /(.*)/; eval "require Inline::$mod;"; warn($@), next if $@; eval "\$register=&Inline::${mod}::register"; @@ -1075,11 +1077,16 @@ sub env_untaint { join ';', grep {not /^\./ and -d $_ } split /;/, $ENV{PATH} : - join ':', grep {/^\// and -d $_ and not ((stat($_))[2] & 0022) + join ':', grep {/^\// and -d $_ and $< == $> ? 1 : not (-W $_ or -O $_) } split /:/, $ENV{PATH}; map {($_) = /(.*)/} @INC; + # list cherry-picked from `perldoc perlrun` + delete @ENV{qw(PERL5OPT PERL5SHELL PERL_ROOT IFS CDPATH ENV BASH_ENV)}; + $ENV{SHELL} = '/bin/sh' if -x '/bin/sh'; + + $< = $> if $< != $>; # so child processes retain euid - ignore failure } #============================================================================== # Blindly untaint tainted fields in Inline object.

Fixed as of 0.55_02