From chris.travers@gmail.com

When relying on the @​INC including the current working directory base no longer follows the same resolution path as use, leading to very complex and difficult to debug error messages. It moreover no longer even follows its existing documentation and this leads to very strange, ordering-dependent bugs which will break a *lot* of software.

Please fix it properly or not at all. This idea of mostly supporting . in the INC but not really is going to cause a lot of people a LOT of headache and lead to systems which are not really any more securable.

See my blog post http​://ledgersmbdev.blogspot.se/2016/07/notes-on-security-separation-of.html on the problems with this fix

From chris.travers@gmail.com

Here is a link to a downstream bug report to Debian based on the same changes​:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=833030

Again, the big problem is that the problems that this patch introduces to this module are very opaque and very difficult to troubleshoot. Frankly removing . from @​INC would be less disruptive and less painful than expecting developers to figure out the strange, ordering dependent bugs that come out of this one.

From zefram@fysh.org

Chris Travers wrote​:

Please fix it properly or not at all. This idea of mostly supporting
. in the INC but not really is going to cause a lot of people a LOT of
headache

We *are* going to fix it properly​: we're heading in the direction of
not having the implicit . in @​INC at all. Expect 5.26 to have at least
a big step in that direction. The problem is that that's quite a big
break in compatibility, which we can't impose straight away. Being a
core change, it also won't help the many programs that run on older
perl versions. So in the short term we need some fixes that address the
really problematic cases without entirely dishonouring . in @​INC, and even
in the long term we need some fixes that don't require the core change.

The changes that you're seeing now are those expedient fixes. Yes,
they're not as good as the proper core change, but they're the best that's
possible to deal with this difficult situation. They're the result
of a balancing act which the security list spent weeks thrashing out.
One of the two classes of immediate change is that code implementing
optional module loads won't honour . in @​INC. This is quite intrusive
to that code, and does cause problems, but really is the least bad thing
we can do to address the serious security problem.

base.pm is especially problematic, because it gets used a lot more than
most optional module loading code, and especially because, due to its
poor design, it mostly gets used for module loads that are not intended
to be optional. We know that this cost arises from the security fix,
and we chose to accept it. It is still less disruptive than an immediate
total removal of . from @​INC would be.

To avoid these problems, you should use parent.pm instead of base.pm.

-zefram

The RT System itself - Status changed from 'new' to 'open'

From zefram@fysh.org

I wrote​:

To avoid these problems, you should use parent.pm instead of base.pm.

And I forgot to say​: if for a particular application you really want a
fully-honoured . in @​INC, you can either put it in at the beginning of
@​INC (by PERL5LIB=., -I., or "use lib '.'"), or put it on the end as
"./.". Eventually you'll need this to load anything from ., when the
core stops putting the . in implicitly.

-zefram

From chris.travers@gmail.com

Removing . from @​INC would frankly cause fewer problems than this modification to base.pm. There are several very significant problems here​:

1. @​INC no longer has a distinct meaning which makes debugging far harder
2. The errors which come from this patch are very opaque, order dependent, and difficult to troubleshoot. Adding Heisenbugs is bad practice.
3. The behavior change and its implications are currently undocumented and probably unknowable in advance.
4. Perhaps most importantly, this patch eliminates the problem from the first places system administrators are likely to look to determine if they are secure while also giving attackers free range of every other module.

In my view, support . in @​INC or not but don't do this funny half-way way which makes undocumented (and probably undocumentable) changes to behavior.

I wear a lot of hats. I do system administration, software development, some pen testing. Debian's rollout of this change broke things that took me a day to debug (because nobody ever expects that problems will come from undocumented behavior changes in standard libraries), and I don't think I will be alone there. When I found out the patch was here too, I filed this bug report.

But then I asked myself, "what is the scope of this vulnerability?" and "Are my systems at risk?" After careful review I determined the business systems I had built were not, but that this was a real problem with a far reach, and that these patches don't actually help that much in securing a system from a sysadmin perspective.

I recognize that this is a difficult situation, and security is important to me too. And I recognize there is probably intense pressure to release a fix yesterday. But this is a bad idea and if you go this route, you will be answering variations of this bug report every day for years to come.

One of the things Debian did right was to introduce a system setting to disable implicit current working directories in @​INC. I recognize that with a portable language like Perl, that may not be practical. But you have a lot of options that would be real solutions to this problem which would be a lot better than what you have now. They include​:

1. A simple compile time option to fix the behavior (woohoo! Perl fixed! now the downstream distributors have to decide whether to release two versions or just the fixed one)

2. Make a fallback if an environment variable is set.

There are probably half a dozen other approaches that would allow for people who still need the old behavior to get it without introducing strange, order-dependent, opaque bugs into large numbers of Perl codebases out there.

A second point is that there are security implications for perl scripts out there and that fixing this on the module level fails to account for the fact that the module is not in a position to understand the implications of the change.

As it stands right now, my view is that system administrators are the only ones who can secure their systems, and that it is critical that they understand the problems in this area. So for that reason, I have started a full disclosure series on this problem discussing different attack scenarios and how system administrators can prevent them (from what to look for in scripts that run on their systems, how to secure scripts, including ones they write themselves, and how to avoid problems from running programs like prove). Every exploit I plan to discuss will work with these patches in place so that system administrators are in a better position to assess vulnerability of their systems.

It is humanly possible to secure a system with this issue. But I am not convinced it is humanly possible to understand the implications of making this the module's responsibility.

From @craigberry

On Mon, Aug 1, 2016 at 10​:13 PM, Chris Travers via RT
<perlbug-followup@​perl.org> wrote​:

Removing . from @​INC would frankly cause fewer problems than this modification to base.pm.

That wasn't the conclusion of the extensive discussions that have
considered that alternative as well as most of the other recommended
courses of action you suggest. I believe most of what's publicly
available is at
<https://rt.perl.org/Public/Bug/Display.html?id=127810>.

I expect some version of removing '.' from @​INC will happen (probably
optional at first). Removal by default right now would have far more
dire consequences than you realize.

As far as things that can go wrong with base.pm being too subtle,
we're doing our best to make the error messages more informative.
Feel free to follow along at
<http​://perl5.git.perl.org/perl.git/history/HEAD​:/dist/base/lib/base.pm>.

From @xsawyerx

Chris, while I understand the objections you have and I appreciate this adds possible complications for you, I think you're missing background and you are making incorrect assumptions.

You are both making assumptions on us not considering things you have (surprise, we have...) and on how easy it would be to solve the problem the way you suggest - which ignores backwards compatibility assurance, different security mechanisms across a multitude of operating systems and file systems, and even code complexity relating to magical entries in @​INC.

I intend to reject this ticket unless I find a reasonable objection.

From chris.travers@gmail.com

On Tue Aug 02 09​:47​:32 2016, xsawyerx@​cpan.org wrote​:

Chris, while I understand the objections you have and I appreciate
this adds possible complications for you, I think you're missing
background and you are making incorrect assumptions.

We fixed our complications after several hours of debugging. My concern is that this is going to break *a lot* of codebases out there and frankly it is *very difficult* to know in advance which ones they will be.

And it makes it harder to determine how secure a system is for the administrator but adds very little complexity for an attacker. The patch makes systems less comprehensibel, adds complex to troubleshoot bugs, and leaves systems with --- AT BEST --- a reason to have a false sense of security.

You are both making assumptions on us not considering things you have
(surprise, we have...) and on how easy it would be to solve the
problem the way you suggest - which ignores backwards compatibility
assurance, different security mechanisms across a multitude of
operating systems and file systems, and even code complexity relating
to magical entries in @​INC.

That's why I suggested a need for a system-wide fallback, which is what Debian wisely did. It enables sysadmins to test their systems, enable the additional security features if and when helpful.

I intend to reject this ticket unless I find a reasonable objection.

Two quick points​:

1. If you are going to change behavior of core modules, it needs to be clearly documented, along with information relating to expected new error conditions and how to fix them. This as not been done and AT A MINIMUM you could treat this as fixed by changing the documentation and loudly announcing likely breakage in advance of release. If you would reject even that modest requirement, then I don't know what to tell you. But at a minimum if you won't restore behavior, then the alternative would be to heavily document *all* expected breakage from this change including error messages and document solutions.

2. As I see it, my concern stands that sysadmins still are the only ones capable of resolving this and only locally currently, and so I will do my best to make sure that the IT professional community understand both the problem and how to prevent it as an attack vector without assuming the safety of any modules. I assume you would agree that this is not anything like a fix that system administrators can rely on for the safety of their systems and so I would hope you would understand that full disclosure is, really, the best that we can do.

From chris.travers@gmail.com

BTW, the other real concern this brings up for someone like myself who has
built a business hosting Perl-based software is this​:

How long before someone panics and breaks something again?

I spent a fair bit of time looking into the issue. I wear many hats --
sysadmin, developer, pen tester, etc and the initial questions were how big
is the problem? Was my business vulnerable? What I did to determine this
was to review the github issues filed over the RFC. I determined that my
business was not vulnerable, but that the problem in other environments
could be very big and frankly very scary. So I am willing to forgive the
panic.

But I also concluded that in terms of exploitability, this approach didn't
really answer a lot of serious problems. For example, because of the way
prove works, it is simply never safe to run while in a world-writable
directory, and that no patches to prove can fix that (that's why it is the
favorite topic of my blog right now, because other sysadmins need to know
what I have found). If you won't tell users that I have to. Will it lower
the bar on writing expoits? Probably. But it also lowers the bar both in
knowledge and effort to securing systems and I think that's more important.

The second thing I determined was that this was trivially preventable on
the script level (no lib '.' for any script intended to run in a directory
of untrusted files unless it is also calling exec on Perl from that same
directory, in which case one just has to note the danger). Perl developers
and sysadmins are in a position to audit their systems, make sure that line
is in place where appropriate, fix he problem locally and report bugs
upstream if not.

In other words, with a day's work or so (what I spent debugging this the
first time around) it is reasonably possible to secure any system including
the effort to report at least some issues upstream.

I have to tell you, I have been primarily writing in Perl 5 since 2007 and
this has really shaken my faith in the Perl community's ability to sit down
and think through security fixes. I don't know if I expect too much. I
understand it is a difficult situation, that maintenance of open source
software is often thankless work, and that there is probably serious
pressure to release a fix yesterday. But this is the first time I have
come to doubt that people would come together when a problem is reported
and actually think through implications of fixes.

You just need a way to choose for now to strip . from the default @​INC.
The sysadmin is the right person to make the call, not the module
maintainer. Keep the system understandable and hence something one can
reasonably secure without special knowledge. Really, that's all I ask.

On Mon, Aug 1, 2016 at 7​:21 PM, Zefram via RT <perlbug-followup@​perl.org>
wrote​:

I wrote​:

To avoid these problems, you should use parent.pm instead of base.pm.

And I forgot to say​: if for a particular application you really want a
fully-honoured . in @​INC, you can either put it in at the beginning of
@​INC (by PERL5LIB=., -I., or "use lib '.'"), or put it on the end as
"./.". Eventually you'll need this to load anything from ., when the
core stops putting the . in implicitly.

-zefram

--
Best Wishes,
Chris Travers

Efficito​: Hosted Accounting and ERP. Robust and Flexible. No vendor
lock-in.
http​://www.efficito.com/learn_more

From chris.travers@gmail.com

On Mon, Aug 1, 2016 at 7​:16 PM, Zefram via RT <perlbug-followup@​perl.org>
wrote​:

Chris Travers wrote​:

Please fix it properly or not at all. This idea of mostly supporting
. in the INC but not really is going to cause a lot of people a LOT of
headache

We *are* going to fix it properly​: we're heading in the direction of
not having the implicit . in @​INC at all.

Good. That would waste people's time a *whole lot less* than the current
fix (which I spent an *entire day* debugging following Debian's roll out).
This is not a proper fix in any universe for a few important reasons​:

1. The bugs it introduces are very opaque and currently the behavior
change is entirely undocumented anywhere.
2. The breaking behavior change (at least in the case of Debian's rollout)
was pretty clearly not understood well enough to alert potential users to
the change in advance. I am not sure this can be understood once INC
sometimes means one thing and sometimes means something else.
3. No amount of this sort of fix can ever secure a system but it can make
admins mistakenly think they are secure by giving false signs in the first
places they might look.
4. Even removing . from @​INC isn't sufficient. In my blog post I discuss
why. You also need to provide a pragma which bans all world writable
directories from @​INC so that programs can at least opt into real security
guarantees.

But by making a code contract only sometimes binding you create extremely
difficult to troubleshoot bugs in peoples' production environments and I
would bet that the cost of that would be worse than the costs of any
exploits.

The first thing to do when faced with a security problem is to make sure
you understand the problem and the solution and I don't think anyone can
understand the full ramifications of this patch. So if it gets in the way
it is, you will answer variations of this bug report every day for years to
come, I would be willing to bet.

Expect 5.26 to have at least
a big step in that direction. The problem is that that's quite a big
break in compatibility, which we can't impose straight away.

So, remove it unless an environment variable is set or something, or make
it a build time option, or half a dozen other things you can do.

Being a
core change, it also won't help the many programs that run on older
perl versions. So in the short term we need some fixes that address the
really problematic cases without entirely dishonouring . in @​INC, and even
in the long term we need some fixes that don't require the core change.

The changes that you're seeing now are those expedient fixes. Yes,
they're not as good as the proper core change, but they're the best that's
possible to deal with this difficult situation.

Well it cost me a day of debugging the fix when it came out in Debian, so
you can expect me to complain loudly about making modules responsible for
INC in this way everywhere this is proposed. It is a bad fix to the wrong
problem and if you stopped and carefully evaluated options I think you'd
see that.

But it also is nowhere near a complete fix, which is why it is critical
that system administrators understand the dangers and how to protect
themselves, so I have started a full disclosure series on this
vulnerability which discusses forms of attacks and how system
administrators can protect themselves against them. I intend to continue
that (my current favorite topic is prove, which cannot be made secure in a
world writeable directory without fundamentally breaking its core
guarantee, unless you remove . from @​INC). Maybe 3-4 published discussions
a week on variations.

Major topics I have planned include prove, scripts that process data in
user-submitted directories, and a few programming antipatterns that make
this problem a lot worse. Like all responsible full disclosure, I intend
to try to ensure that the cost in lowering the barrier to writing exploits
is small in comparison to the benefit to sysadmins trying to secure their
systems.

Because right now system administrators can reasonably protect their
systems. The current approach given here does not help with that in any
real significant way.

They're the result
of a balancing act which the security list spent weeks thrashing out.
One of the two classes of immediate change is that code implementing
optional module loads won't honour . in @​INC. This is quite intrusive
to that code, and does cause problems, but really is the least bad thing
we can do to address the serious security problem.

base.pm is especially problematic, because it gets used a lot more than
most optional module loading code, and especially because, due to its
poor design, it mostly gets used for module loads that are not intended
to be optional. We know that this cost arises from the security fix,
and we chose to accept it. It is still less disruptive than an immediate
total removal of . from @​INC would be.

Then I hope *at least* you announce the breakage in a big way, properly
document it in the POD (right now that isn't there), and make sure that
everyone who faces the dreaded base class empty error knows that this is
because of this change. Nobody should have to spend a day debugging only
to find the problem is an undocumented change in behavior to a part of the
standard module. I won't get back the day that I would have spent doing
billable work. Neither will my business partner. But at least you can try
to minimize that from happening to everyone else.

If you would like to follow the full disclosure entries I will email you
directly. I don't see a point in putting them in the ticket.

To avoid these problems, you should use parent.pm instead of base.pm.

-zefram

--
Best Wishes,
Chris Travers

Efficito​: Hosted Accounting and ERP. Robust and Flexible. No vendor
lock-in.
http​://www.efficito.com/learn_more

From zefram@fysh.org

You have misunderstood the vulnerability. "prove foo.pl" is vulnerable
only in the same way as "sh foo.sh"​: sure, in a world-writable directory
the requested file could be replaced by an attacker, but that's inherent
in what the user requested. We do not regard that as a vulnerability in
prove or in perl. A particular test script can be vulnerable, depending
on its module usage, but that's specific to the test script, not a general
problem with prove. Likewise, while world-writable directories in @​INC
are certainly a problem, they've generally got there by user request,
through PERL5LIB or -I. That's an issue worth blogging about, but it's
not the security issue at work here.

The problem is specifically with the . that exists in @​INC by default.
It's a problem because the user hasn't requested it. The problem
is also specifically with optional module loads. If a program loads
modules on a mandatory basis, then we expect that when it's installed
those modules will be installed, so the program's operation won't ever
reach the . in @​INC. The problem is with programs that load modules
optionally, and so might be installed without them, in which case the
. in @​INC would routinely be used when the program runs.

In the absence of a global change to the initialisation of @​INC, the
best solution is for the program at top level to remove the implicit
. from @​INC. Essentially every Perl program ought to do this. We are
in fact making that change to Perl programs that we control.

The reason why we're also doing the much nastier @​INC twiddling in modules
that perform optional module loading is to get sufficient coverage.
It's far too easy for Perl programs to slip through, never getting the
fix at top level. This is especially true if the program doesn't know at
top level that there will be some optional module loading by some module
that it uses. Most optional module loading is performed by modules,
so by applying patches there we can cover most vulnerable module loading
even in the presence of Perl programs whose authors aren't fully aware
of the issue.

I agree with you on the matter of documentation. We do need to make the
compatibility issue clearer in perldelta and in the base.pm documentation.
We also need to make the issue with base.pm more discoverable than it
was in the RC, and you'll be pleased to hear that there has been a big
improvement in diagnostics since the RC.

-zefram

From chris.travers@gmail.com

Zefram​:

No, you misunderstand me. The point there is that prove is a nice way to demonstrate the vulnerability in test scripts and that test frameworks must be insecure by design (because otherwise they would block problems found in production systems.

Test frameworks are arbitrary code execution frameworks and it is important for administrators to secure them assuming that scripts run during the tests are vulnerable. That's my point. The focus of these posts is to look at full attack scenarios and what administrators can do to stop them. Surely you would agree that administrators running test systems should assume that tested code may do require inside an eval. Would you not agree?

My next blog post will be on the question of the security of pl/perlU stored procedures and user defined functions running in PostgreSQL.

From chris.travers@gmail.com

There are also, as it occurs to me, a number of things that could be done to make these sorts of changes (even if you insist on moving to nebulous software contracts) far more tolerable to developers. These include​:

1. A real effort to make the failure cases transparent. For example, failing to require without . you could check the current directory for the file and warn if it exists. That would make debugging these sorts of problems easy and even more valuable than changes docs.

2. Clear documentation describing the sorts of errors that result from this change and their solutions.

At least with the Debian backport (and the RC2 tarball) these things are not done. base.pm does not conform to documented behavior or to past behavior and therefore has a serious bug. You can address the bug in any of a number of ways (and documentation/transparency of error conditions is one) but it needs to be addressed.

From @cpansprout

On Tue Aug 02 21​:53​:07 2016, chris.travers@​gmail.com wrote​:

There are also, as it occurs to me, a number of things that could be
done to make these sorts of changes (even if you insist on moving to
nebulous software contracts) far more tolerable to developers. These
include​:

1. A real effort to make the failure cases transparent.

That was my complaint in ticket #15484

For example,
failing to require without . you could check the current directory for
the file and warn if it exists.

Which is exactly the change I have made to base.pm. Something like my patch will be included in 5.24.1 proper.

--

Father Chrysostomos

From chris.travers@gmail.com

On Wed, Aug 3, 2016 at 8​:08 AM, Father Chrysostomos via RT <
perlbug-followup@​perl.org> wrote​:

On Tue Aug 02 21​:53​:07 2016, chris.travers@​gmail.com wrote​:

There are also, as it occurs to me, a number of things that could be
done to make these sorts of changes (even if you insist on moving to
nebulous software contracts) far more tolerable to developers. These
include​:

1. A real effort to make the failure cases transparent.

That was my complaint in ticket #128769.

Great :-)

For example,
failing to require without . you could check the current directory for
the file and warn if it exists.

Which is exactly the change I have made to base.pm. Something like my
patch will be included in 5.24.1 proper.

Also good news!

--

Father Chrysostomos

--
Best Wishes,
Chris Travers

Efficito​: Hosted Accounting and ERP. Robust and Flexible. No vendor
lock-in.
http​://www.efficito.com/learn_more

I don't see any further action needed here. Chris seems to be happy. I think this ticket can be closed.

I don't see any further action needed here. Chris seems to be happy. I think this ticket can be closed.

+1. Closing.

Thank you very much.
Jim Keenan