Sat Jun 21 18:47:52 2014: Request 96291 was acted upon.
Transaction: Correspondence added by ETJ
Queue: Inline
Subject: t/08taint.t fails on perl 5.20.0
Broken in: 0.55
Severity: (no value)
Owner: Nobody
Requestors: [email protected]
Status: open
Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=96291 >
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;
