Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

[rt.cpan.org #96291] t/08taint.t fails on perl 5.20.0

3 views
Skip to first unread message

Ed J via RT

unread,
Jun 6, 2014, 9:34:29 PM6/6/14
to inl...@perl.org
Fri Jun 06 21:34:28 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: E...@cpan.org
Status: new
Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=96291 >


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.

sisy...@optusnet.com.au

unread,
Jun 6, 2014, 9:39:53 PM6/6/14
to bug-I...@rt.cpan.org, inl...@perl.org


-----Original Message-----
From: Ed J via RT

> It says (on my system): "sh: make: command not found".

Yes, this happens on some systems.
The problem has not yet been fixed - see

https://rt.cpan.org/Ticket/Display.html?id=65703

for a fuller discussion. I think there are some workarounds mentioned in
there if you're actually wanting to run Inline taint-activated.
If you're not wanting to run Inline with taint turned on, just "force"
install the module.

Note that this failure simply signifies that you can't run Inline under
taint. It doesn't impact on any other aspects of the module.

Cheers,
Rob


sisyphus1@optusnet.com.au via RT

unread,
Jun 6, 2014, 9:41:10 PM6/6/14
to inl...@perl.org
Fri Jun 06 21:41:09 2014: Request 96291 was acted upon.
Transaction: Correspondence added by sisy...@optusnet.com.au
Queue: Inline
Subject: Re: [rt.cpan.org #96291] t/08taint.t fails on perl 5.20.0
Broken in: 0.55
Severity: (no value)
Owner: Nobody
Requestors: E...@cpan.org
Status: new
Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=96291 >




sisy...@optusnet.com.au

unread,
Jun 7, 2014, 3:51:56 AM6/7/14
to bug-I...@rt.cpan.org, inl...@perl.org
-----Original Message-----
From: Ed J via RT

> 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.

Really ? I thought it was purely dependent upon system configuration, and
completely independent of perl version.
On my Windows 7, Ubuntu 12.04, and Debian Wheezy systems the 08taint.t tests
pass (for perl-5.20.0 as well as earlier versions of perl).

> I hoped there would be a sensible value available in %Config, but there
> isn't.

I would happily dismantle Inline's attempted taint handling if:
a) Ingy gives his blessing for that to happen;
&&
b) there's a consensus that this is the right thing to do.

So far neither has happened.
In the meantime, patches are welcome.

I guess there are other things we could do - eg skip the 08taint.t test
script if (eg) $ENV{INLINE_NTT} was set. ("NTT" being a mnemonic for "No
Taint Testing").
I've no objection to doing that. In fact, I think I might do just that - it
comes at no cost to those who don't want to make use of the option.

However, I don't think I would like to force those tests to be skipped for
5.20. Someone might not notice that - and then get really annoyed because
the test suite didn't disclose to them that taint did not work.

Cheers,
Rob

sisyphus1@optusnet.com.au via RT

unread,
Jun 7, 2014, 3:53:07 AM6/7/14
to inl...@perl.org
Sat Jun 07 03:53:07 2014: Request 96291 was acted upon.
Transaction: Correspondence added by sisy...@optusnet.com.au
Queue: Inline
Subject: Re: [rt.cpan.org #96291] t/08taint.t fails on perl 5.20.0
Broken in: 0.55
Severity: (no value)
Owner: Nobody
Requestors: E...@cpan.org
Status: open
Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=96291 >

Ed . via RT

unread,
Jun 7, 2014, 1:15:46 PM6/7/14
to inl...@perl.org
Sat Jun 07 13:15:45 2014: Request 96291 was acted upon.
Transaction: Correspondence added by ej...@hotmail.com
Queue: Inline
Subject: Re: [rt.cpan.org #96291] t/08taint.t fails on perl 5.20.0
Broken in: 0.55
Severity: (no value)
Owner: Nobody
Requestors: E...@cpan.org
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

-----Original Message-----
From: sisy...@optusnet.com.au via RT
Sent: Saturday, June 07, 2014 8:53 AM
To: E...@cpan.org
Subject: Re: [rt.cpan.org #96291] t/08taint.t fails on perl 5.20.0

<URL: https://rt.cpan.org/Ticket/Display.html?id=96291 >

sisy...@optusnet.com.au

unread,
Jun 8, 2014, 5:08:35 AM6/8/14
to bug-I...@rt.cpan.org, inl...@perl.org
-----Original Message-----
From: Ed . via RT

> 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)

Yes, CPAN.pm and friends are quite deficient in the way they handle test
failures. I don't like them and avoid them (except for very long dependency
chains) for that and other reasons.
IMO, if there's a test failure, they should prompt you as to whether the
module should be installed - not just make you re-run the process with
force.
Perhaps there's already an option for them to do that. If there's not such
an option, then maybe you should complain to the developers of those
modules.

> Do you approve of this strategy?

Not sure about step 2.
If the test succeeds only because 08taint.t sets $PATH to '/bin:/usr/bin',
then it has passed because we've rigged the test. We have deceived the user
into thinking that taint enabling in Inline works straight out of the box -
which is not the case (as he must first set $PATH appropriately).

Can we do just steps 3 and 4 ? If we can detect that the 08taint.t test is
bound to fail because 'make' and/or $Config{cc} are not going to be found,
then I'll accept that we skip the test.
If you've got a patch that performs that detection and then skips the tests,
send it out and I'll apply it.

In those cases where 08taint.t is then skipped, we need to bellow out that
"INLINE WILL NOT RUN UNDER -T ON THIS SYSTEM" ... and we probably also need
to alter the documentation.
But I can take care of those aspects.

It's not really the right thing to do.
I mean, the idea is that if 08taint.t fails then the user should make the
call on whether the module gets installed. With your proposed changes, the
decision to install has already been made for him .... and he doesn't even
know that his Inline is not capable of running under -T (unless he was
paying close attention to the build output).
But I'm ready to go with that approach anyway :-)

Alternatively, if you like, I'm now prepared to turn off the 08taint.t by
default, and have it run only if $ENV{INLINE_TT_ON} is set.
That's not the right thing to do either, but I simply don't want to continue
having to devote attention to a feature of Inline that no-one uses, and that
should never have been created in the first place.
It has been ongoing for way too long.

Cheers,
Rob

sisyphus1@optusnet.com.au via RT

unread,
Jun 8, 2014, 5:09:36 AM6/8/14
to inl...@perl.org
Sun Jun 08 05:09:33 2014: Request 96291 was acted upon.
Transaction: Correspondence added by sisy...@optusnet.com.au
Queue: Inline
Subject: Re: [rt.cpan.org #96291] t/08taint.t fails on perl 5.20.0
Broken in: 0.55
Severity: (no value)
Owner: Nobody
Requestors: E...@cpan.org
Status: open
Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=96291 >


Michael Conrad via RT

unread,
Jun 8, 2014, 11:59:43 PM6/8/14
to inl...@perl.org
Sun Jun 08 23:59:42 2014: Request 96291 was acted upon.
Transaction: Correspondence added by NERDVANA
Queue: Inline
Subject: t/08taint.t fails on perl 5.20.0
Broken in: 0.55
Severity: (no value)
Owner: Nobody
Requestors: E...@cpan.org
Status: open
Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=96291 >


On Fri Jun 06 20:50:00 2014, ETJ wrote:
> It says (on my system): "sh: make: command not found".
>
> A little instrumentation in the "make" method indicated its $ENV{PATH}
> was empty, which sort of makes sense as a secure thing to do, but
> doesn't seem to offer any obvious place for a workaround.


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.

Michael Conrad via RT

unread,
Jun 9, 2014, 12:02:52 AM6/9/14
to inl...@perl.org
Mon Jun 09 00:02:51 2014: Request 96291 was acted upon.
Transaction: Correspondence added by NERDVANA
Queue: Inline
Subject: t/08taint.t fails on perl 5.20.0
Broken in: 0.55
Severity: (no value)
Owner: Nobody
Requestors: E...@cpan.org
Status: open
Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=96291 >


Oh, and the perls involved were 5.12.4 and 5.16.3, so it isn't specific to 5.20

Mahmoud Mehyar via RT

unread,
Jun 9, 2014, 12:15:21 AM6/9/14
to inl...@perl.org
Mon Jun 09 00:15:21 2014: Request 96291 was acted upon.
Transaction: Correspondence added by mamod....@gmail.com
Queue: Inline
Subject: Re: [rt.cpan.org #96291] t/08taint.t fails on perl 5.20.0
Broken in: 0.55
Severity: (no value)
Owner: Nobody
Requestors: E...@cpan.org
Status: open
Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=96291 >


In my case taint test always fail on all platforms I tried to install on,
ubuntu and freeBSD I don't remember if that was the same with windows but I
started to use force install every time I update or install Inline as a
habit :)

Michael Conrad via RT

unread,
Jun 9, 2014, 1:00:04 AM6/9/14
to inl...@perl.org
Mon Jun 09 01:00:03 2014: Request 96291 was acted upon.
Transaction: Correspondence added by NERDVANA
Queue: Inline
Subject: t/08taint.t fails on perl 5.20.0
Broken in: 0.55
Severity: (no value)
Owner: Nobody
Requestors: E...@cpan.org
Status: open
Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=96291 >


On Mon Jun 09 00:02:51 2014, NERDVANA wrote:
> Oh, and the perls involved were 5.12.4 and 5.16.3, so it isn't
> specific to 5.20

I have further discovered that it only happens when I run cpan or cpanm as root. When I run "make test" manually as a normal user (with the files chown'd to that user) the test passes.

sisy...@optusnet.com.au

unread,
Jun 9, 2014, 11:00:17 AM6/9/14
to bug-I...@rt.cpan.org, inl...@perl.org
-----Original Message-----
From: Mahmoud Mehyar via RT

> In my case taint test always fail on all platforms I tried to install on,
> ubuntu and freeBSD
> I don't remember if that was the same with windows

I don't think I've ever seen a report of it having failed on Windows -
though I suppose it may be possible to set things up on Windows so that the
failure happens.

> but I started to use force install every time I update or install Inline
> as a habit :)

That's probably another reason we should turn off testing of t/08taint.t.
One day something more important than t/08taint.t might also fail and you
won't notice ... and Inline will still be installed anyway. (I guess if it's
"something important" you'll *eventually* discover the failure ;-)

I don't know of anyone that wants this taint handling fixed so that they can
actually make use of it. AFAICT people want it fixed only so that they can
avoid using force when installing Inline with any of the "cpan" tools.

So I think I *will* turn t/08taint.t testing off unless $ENV{INLINE_TT_ON}
is set ... and see what results/reactions that produces.
I should never have added t/08taint.t in the first place. I should have just
left the taint handling in its completely broken state. (No-one would ever
have known.)

Cheers,
Rob

sisyphus1@optusnet.com.au via RT

unread,
Jun 9, 2014, 11:01:16 AM6/9/14
to inl...@perl.org
Mon Jun 09 11:01:15 2014: Request 96291 was acted upon.
Transaction: Correspondence added by sisy...@optusnet.com.au
Queue: Inline
Subject: Re: [rt.cpan.org #96291] t/08taint.t fails on perl 5.20.0
Broken in: 0.55
Severity: (no value)
Owner: Nobody
Requestors: E...@cpan.org
Status: open
Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=96291 >


Reini Urban

unread,
Jun 9, 2014, 4:47:52 PM6/9/14
to inl...@perl.org
This is then a serious user problem.
Tests should never be run as root, way too dangerous.
The cpan install steps for EUMM and MB have usually the necessary sudo
prepended.
I haven't checked if cpanm --sudo is broken as I never use it, but the
docs day it's used only for install, which is good.

In our case I suggest to set the empty tainted PATH to /bin:/usr/bin
and make the tests TODO.
On cygwin this happens e.g. if those paths are group writable of if you
run the tests as root.

Skipping is bad, since some user might want to use Inline C with tainted
input, and will not see new problems then.

--
Reini

Working towards a true Modern Perl.
Slim, functional, unbloated, compile-time optimizable

Michael Conrad via RT

unread,
Jun 9, 2014, 5:40:40 PM6/9/14
to inl...@perl.org
Mon Jun 09 17:40:39 2014: Request 96291 was acted upon.
Transaction: Correspondence added by NERDVANA
Queue: Inline
Subject: t/08taint.t fails on perl 5.20.0
Broken in: 0.55
Severity: (no value)
Owner: Nobody
Requestors: E...@cpan.org
Status: open
Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=96291 >


On Mon Jun 09 11:01:15 2014, sisy...@optusnet.com.au wrote:
> I don't know of anyone that wants this taint handling fixed so that
> they can
> actually make use of it. AFAICT people want it fixed only so that they
> can
> avoid using force when installing Inline with any of the "cpan" tools.
>
> So I think I *will* turn t/08taint.t testing off unless
> $ENV{INLINE_TT_ON}
> is set ... and see what results/reactions that produces.
> I should never have added t/08taint.t in the first place. I should
> have just
> left the taint handling in its completely broken state. (No-one would
> ever
> have known.)
>
> Cheers,
> Rob

Sounds fine by me.

But in case anyone ever comes asking for proper handling with taint mode, here's a quick way to reproduce a system where it fails, using a Gentoo chroot:

wget http://distfiles.gentoo.org/releases/amd64/autobuilds/current-stage3-amd64-nomultilib/stage3-amd64-nomultilib-20140529.tar.bz2
mkdir gentoo-chroot
tar -xjf stage3-amd64-nomultilib-20140529.tar.bz2 -C gentoo-chroot
mount --rbind /dev gentoo-chroot/dev
mount --bind /proc gentoo-chroot/proc
cp /etc/resolv.conf gentoo-chroot/etc/
chroot gentoo-chroot /bin/bash
source /etc/profile
sed -ie 's/^#en_US/en_US/' /etc/locale.gen
locale-gen
cpan Inline

Ed J via RT

unread,
Jun 9, 2014, 7:53:54 PM6/9/14
to inl...@perl.org
Mon Jun 09 19:53:53 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: E...@cpan.org
Status: open
Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=96291 >


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.

sisy...@optusnet.com.au

unread,
Jun 9, 2014, 9:40:47 PM6/9/14
to Reini Urban, inl...@perl.org
-----Original Message-----
From: Reini Urban

> In our case I suggest to set the empty tainted PATH to /bin:/usr/bin

Where do we do that ?
If we do it in the test script, and the script then passes, the user will
think Inline works under -T.
The user then writes a script that runs under -T only to find it doesn't
work, because he hasn't set the empty PATH to /bin:/usr/bin (and there's no
reason that he should know that's needed).

So ... this check needs to be done in Inline.pm.
So ... at the end of sub env_untaint do we do something like:

$ENV{PATH} = '/bin:/usr/bin' unless $ENV{PATH};

(I'm assuming that, in those cases where $ENV{PATH} gets completely emptied
out, it's happening in env_untaint(). Is that right ?)

Is that a completely safe and harmless thing to do - or should we emit a
warning along the lines of "Setting empty \$PATH to '/bin:/usr/bin'" ?

> and make the tests TODO.
>
> Skipping is bad, since some user might want to use Inline C with tainted
> input, and will not see new problems then

Ok - TODO will do. It's just as effective as SKIP at sweeping problems
under the carpet. (And, you're right, TODO does make it possible to detect a
change in behaviour.)

Do you mean that we make it a blanket TODO for all systems and situations ?
... or do you mean make it TODO only if $ENV{PATH} was empty ?

If it's the latter, then we need a way for t/08taint.t to detect that
Inline.pm set the empty $PATH to '/bin:/usr/bin'. (Just set a
$INLINE::path_fiddle variable to true iff we've replaced an empty $PATH with
'/bin:/usr/bin'.)

Cheers,
Rob

sisy...@optusnet.com.au

unread,
Jun 9, 2014, 9:59:24 PM6/9/14
to bug-I...@rt.cpan.org, inl...@perl.org
-----Original Message-----
From: Ed J via RT

> If no-one else wants to, I'll do both the test-possible-skipping and a doc
> update?

Ok - thanks for the offer.
See my response (posted just a few minutes ago) to Reini's post in this
thread.

I think, go with your original 4-point plan:

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)

But steps 1 and 2 need to be taken inside Inline.pm (not inside
t/08taint.t).
And replace 'skip' with 'todo' in step 4.

I think there should be no need for any changes to the documentation as a
result of that ... but feel free to make any documentary changes that you
see fit.

If you can do that, I think it would be most helpful. (I'm a bit pressed for
time .... and also don't have machine that exhibits the problem, for
testing.)

> It would probably be a candidate for a fast new release.

Yep - early next week I could make a devel release, followed by a new stable
release a week later, all being well.
(I can always find time to make another release.)

Thanks for pursuing and persisting ;-)

Cheers,
Rob

sisyphus1@optusnet.com.au via RT

unread,
Jun 9, 2014, 10:00:03 PM6/9/14
to inl...@perl.org
Mon Jun 09 22:00:02 2014: Request 96291 was acted upon.
Transaction: Correspondence added by sisy...@optusnet.com.au
Queue: Inline
Subject: Re: [rt.cpan.org #96291] t/08taint.t fails on perl 5.20.0
Broken in: 0.55
Severity: (no value)
Owner: Nobody
Requestors: E...@cpan.org
Status: open
Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=96291 >


Ed J via RT

unread,
Jun 11, 2014, 5:52:25 AM6/11/14
to inl...@perl.org, sisy...@optusnet.com.au, mamod....@gmail.com
<URL: https://rt.cpan.org/Ticket/Display.html?id=96291 >

On Mon Jun 09 01:00:03 2014, NERDVANA wrote:
> On Mon Jun 09 00:02:51 2014, NERDVANA wrote:
> > Oh, and the perls involved were 5.12.4 and 5.16.3, so it isn't
> > specific to 5.20
>
> I have further discovered that it only happens when I run cpan or
> cpanm as root. When I run "make test" manually as a normal user (with
> the files chown'd to that user) the test passes.

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.

Ed J via RT

unread,
Jun 11, 2014, 5:52:25 AM6/11/14
to inl...@perl.org
Wed Jun 11 05:52:23 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: E...@cpan.org
Status: open
Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=96291 >

Sisyphus via RT

unread,
Jun 20, 2014, 11:56:25 PM6/20/14
to inl...@perl.org
Fri Jun 20 23:56:25 2014: Request 96291 was acted upon.
Transaction: Correspondence added by SISYPHUS
Queue: Inline
Subject: t/08taint.t fails on perl 5.20.0
Broken in: 0.55
Severity: (no value)
Owner: Nobody
Requestors: E...@cpan.org
Status: open
Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=96291 >


Inline-0.55_01 was released a couple of days ago, partly to see how t/08taint.t fares following the latest round of amendments to the Inline code.

I've just received the first FAIL report for 0.55_01:
http://www.cpantesters.org/cpan/report/a3185cf8-f82f-11e3-919b-572b4803b899

The culprit is, of course, C/t/08taint.t.

What to do wrt this ?

If it's fixable, and we can fix it, then I guess we should do that.
If we can't fix it, then we need to detect in advance that the failure will occur and either SKIP or TODO the remaining 9 tests.

There's a couple of issues with taking the TODO route:

1) The script is actually dying before all tests have been run - so I don't think TODO'ing the remaining tests is going to prevent the script from being reported as a FAIL. (We can probably workaround that by running the remaining tests inside eval{}.)

2) 3 of the tests are warnings_like() from Test::Warn, and it seems that TODO is ignored for them. That being so, we need to work around that shortcoming, too. (I was unable to find a way to make those 3 tests honor TODO - but that doesn't necessarily mean it can't be done.)

However, I think that if there's no chance of Inline being able to run correctly under -T on a particular system, then we are quite entitled to SKIP those tests on that system.

Thoughts ?

Cheers,
Rob


Ed J via RT

unread,
Jun 21, 2014, 6:47:52 PM6/21/14
to inl...@perl.org
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: E...@cpan.org
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;

Ed J via RT

unread,
Jun 21, 2014, 7:22:12 PM6/21/14
to inl...@perl.org
Sat Jun 21 19:22:11 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: E...@cpan.org
Status: open
Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=96291 >


On further reflection, the previous logic and patch is slightly imperfect; a malicious user could include a directory under their control, put in a "chmod" command, then deny themselves write permission, and the directory would still be permitted. Instead, this patch, which replaces the previous one, will strip out directories either writable OR owned by the real uid:

diff --git a/Inline.pm b/Inline.pm
index 32868a3..3b62337 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 $_ or -O $_)

Ed J via RT

unread,
Jun 21, 2014, 10:28:51 PM6/21/14
to inl...@perl.org
Sat Jun 21 22:28:50 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: E...@cpan.org
Status: open
Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=96291 >


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.

Sisyphus via RT

unread,
Jun 22, 2014, 7:05:26 AM6/22/14
to inl...@perl.org
Sun Jun 22 07:05:25 2014: Request 96291 was acted upon.
Transaction: Correspondence added by SISYPHUS
Queue: Inline
Subject: t/08taint.t fails on perl 5.20.0
Broken in: 0.55
Severity: (no value)
Owner: Nobody
Requestors: E...@cpan.org
Status: open
Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=96291 >


On Sat Jun 21 22:28:50 2014, ETJ wrote:

> Instead, this patch (replacing previous two) actually
> tests $< != $>, which revealed a couple of quirks, hence a couple of
> extra changes:

Thanks Ed - checks out ok here, so I've applied the patches and released Inline-0.55_02.

On Windows both $< and $> are 0 and cannot be altered as seteuid() and setruid() are not implemented.
In C/t/08taint.t I therefore had to make the setting of $< to 1 conditional upon OS not being Windows (to avoid fatal error on that OS).

With that in place, everything tests fine for me on Windows, Cygwin, Ubuntu and Debian Wheezy.
Now we sit back and see what the smokers/testers throw at us :-)

Cheers,
Rob

Sisyphus via RT

unread,
Jun 24, 2014, 5:01:45 AM6/24/14
to inl...@perl.org
Tue Jun 24 05:01:44 2014: Request 96291 was acted upon.
Transaction: Correspondence added by SISYPHUS
Queue: Inline
Subject: t/08taint.t fails on perl 5.20.0
Broken in: 0.55
Severity: (no value)
Owner: Nobody
Requestors: E...@cpan.org
Status: open
Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=96291 >



Fixed as of 0.55_02
0 new messages