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

[Ed J via RT]

Fri Jun 06 20:50:00 2014: Request 96291 was acted upon.
Transaction: Ticket created by ETJ
Queue: Inline
Subject: t/08taint.t fails on perl 5.20.0
Broken in: 0.55
Severity: (no value)
Owner: Nobody
Requestors: ETJ-***@public.gmane.org
Status: new
Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=96291 >


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.

[Ed J via RT]

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: ETJ-***@public.gmane.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.

[s***@public.gmane.org]

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

Post by 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).

Post by Ed J via RT
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-sFbbPxZDHXw0n/[e-mail] via RT]

Sat Jun 07 03:53:07 2014: Request 96291 was acted upon.
Transaction: Correspondence added by sisyphus1-sFbbPxZDHXw0n/***@public.gmane.org
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: ETJ-***@public.gmane.org
Status: open
Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=96291 >


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

Post by 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).

Post by Ed J via RT
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

[Ed . via RT]

Sat Jun 07 13:15:45 2014: Request 96291 was acted upon.
Transaction: Correspondence added by ej_zg-***@public.gmane.org
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: ETJ-***@public.gmane.org
Status: open
Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=96291 >


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: sisyphus1-sFbbPxZDHXw0n/***@public.gmane.org via RT
Sent: Saturday, June 07, 2014 8:53 AM
To: ETJ-***@public.gmane.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 >

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

Post by 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).

Post by Ed J via RT
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

[s***@public.gmane.org]

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

Post by Ed . via RT
Per the discussion with mst on #perl (ex pumpkin holder), I propose (and
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.

Post by Ed . via RT
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-sFbbPxZDHXw0n/[e-mail] via RT]

Sun Jun 08 05:09:33 2014: Request 96291 was acted upon.
Transaction: Correspondence added by sisyphus1-sFbbPxZDHXw0n/***@public.gmane.org
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: ETJ-***@public.gmane.org
Status: open
Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=96291 >


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

Post by Ed . via RT
Per the discussion with mst on #perl (ex pumpkin holder), I propose (and
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.

Post by Ed . via RT
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

[s***@public.gmane.org]

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

Post by 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-sFbbPxZDHXw0n/[e-mail] via RT]

Fri Jun 06 21:41:09 2014: Request 96291 was acted upon.
Transaction: Correspondence added by sisyphus1-sFbbPxZDHXw0n/***@public.gmane.org
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: ETJ-***@public.gmane.org
Status: new
Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=96291 >




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

Post by 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

[Michael Conrad via RT]

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: ETJ-***@public.gmane.org
Status: open
Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=96291 >

Post by Ed J via RT
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.

[Mahmoud Mehyar via RT]

Mon Jun 09 00:15:21 2014: Request 96291 was acted upon.
Transaction: Correspondence added by mamod.mehyar-***@public.gmane.org
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: ETJ-***@public.gmane.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 :)


On Mon, Jun 9, 2014 at 6:59 AM, Michael Conrad via RT <

Post by Michael Conrad via RT
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
Status: open
Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=96291 >

Post by Ed J via RT
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.

[s***@public.gmane.org]

-----Original Message-----
From: Mahmoud Mehyar via RT

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

Post by Mahmoud Mehyar via RT
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-sFbbPxZDHXw0n/[e-mail] via RT]

Mon Jun 09 11:01:15 2014: Request 96291 was acted upon.
Transaction: Correspondence added by sisyphus1-sFbbPxZDHXw0n/***@public.gmane.org
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: ETJ-***@public.gmane.org
Status: open
Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=96291 >


-----Original Message-----
From: Mahmoud Mehyar via RT

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

Post by Mahmoud Mehyar via RT
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

[Michael Conrad via RT]

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: ETJ-***@public.gmane.org
Status: open
Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=96291 >

Post by s***@public.gmane.org
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]

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: ETJ-***@public.gmane.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.

[sisyphus1-sFbbPxZDHXw0n/[e-mail] via RT]

Mon Jun 09 22:00:02 2014: Request 96291 was acted upon.
Transaction: Correspondence added by sisyphus1-sFbbPxZDHXw0n/***@public.gmane.org
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: ETJ-***@public.gmane.org
Status: open
Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=96291 >


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

Post by 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.)

Post by Ed J via RT
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

[s***@public.gmane.org]

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

Post by 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.)

Post by Ed J via RT
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

[Michael Conrad via RT]

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: ETJ-***@public.gmane.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

[Michael Conrad via RT]

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: ETJ-***@public.gmane.org
Status: open
Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=96291 >

Post by Michael Conrad via RT
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.

[Reini Urban]

Post by Michael Conrad via RT
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
Status: open
Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=96291 >

Post by Michael Conrad via RT
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.

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.

[s***@public.gmane.org]

-----Original Message-----
From: Reini Urban

Post by 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'" ?

Post by Reini Urban
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

[Ed J via RT]

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

Post by Michael Conrad via RT

Post by Michael Conrad via RT
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]

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: ETJ-***@public.gmane.org
Status: open
Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=96291 >

Post by Michael Conrad via RT

Post by Michael Conrad via RT
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.

[Sisyphus via RT]

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: ETJ-***@public.gmane.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]

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: ETJ-***@public.gmane.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]

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: ETJ-***@public.gmane.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 $_)
} split /:/, $ENV{PATH};

map {($_) = /(.*)/} @INC;

[Ed J via RT]

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: ETJ-***@public.gmane.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]

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: ETJ-***@public.gmane.org
Status: open
Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=96291 >

Post by Ed J via RT
Instead, this patch (replacing previous two) actually
tests $< != $>, which revealed a couple of quirks, hence a couple of

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]

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: ETJ-***@public.gmane.org
Status: open
Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=96291 >



Fixed as of 0.55_02