Notes on Security, Separation of Concerns and CVE-2016-1238 (Full Disclosure)

A cardinal rule of software security is that, when faced with a problem, make sure you fully understand it before implementing a fix.  A cardinal rule of general library design is that heuristic approaches to deciding whether something is a problem really should be disfavored.  These lessons were driven home when I ended up spending a lot of time debugging problems caused by a recent Debian fix for the CVE in the title.  Sure everyone messes up sometimes, and so this isn't really condemnation of Debian as a project but their handling of this particular CVE is a pretty good example of what not to do.

In this article I am going to discuss actual exploits.  Full disclosure has been a part of the LedgerSMB security culture since we started and discussion of exploits in this case provide administrators with real chances to secure their systems, as well as distro maintainers, Perl developers etc to write more secure software.  Recommendations will be further given at the end regarding improving the security of Perl as a programming language.

Part of the problem in this case is that the CVE is poorly scoped but CVE's are often poorly scoped and it is important for developers to work with security researchers to understand a problem, understand the implications of different approaches to fix it and so forth.  It is very easy to get in a view that "this must be fixed right now" but all too often (as here) a shallow fix does not completely resolve an issue and causes more problems than it resolves.

The Problem (with exploits)

Perl's system of module inclusion (and other operations) looks for Perl modules in the current directory after exhausting other directories.  Technically this is optional but most UNIX and Linux distributions have this behavior.  On the whole it is bad practice as well, which is why it is not by the default behavior of shells like bash.  But a lot of software depends on this (with some legitimate use cases) and so changing it is problematic.

Perl programs are also often complex and have optional dependencies which may or may not exist on a system.  If those do not otherwise exist in the system Perl directories but exist in the current working directory, then these may be loaded from the current working directory.  Note that this is not actually limited to the current working directory, and Perl could load the files from all kinds of places, users-specified or not.

So when one is running a Perl program in a world-writeable location, there is the opportunity for another user to put code there that may be picked up by the Perl interpreter and executed.  While the CVE is limited to implicit inclusion of the current working directory, the problem is actually quite a bit broader than that.  Include paths can be specified on the command line and if any of them are world-writeable, then variations of the same attacks are possible.

Some programs, of course, are intended to run arbitrary Perl code.  The test harness programs are good examples of this.  Special attention will be given to the ways test harness programs can be exploited here.

These features come together to create opportunities for exploits in multi-user systems which administrators need to be aware of and take immediate steps to prevent.  In my view there are a few important and needed features in Perl as well.

A simple exploit:

Create the following files in a safe directory:

t/01-security.t, contents:

use Test::More;
require bar;
eval { require foo };

plan skip_all => 'nothing to do';

lib/bar.pm contents:

use 5.010;
warn "this is ok";

./foo.pm, contents:

use 5.010;
warn "Haxored!";

now run:

prove -Ilib t/01-security.t

Now, what happens here is that the optional requirement of foo.pm in the test script gets resolved to the one that happens to be in your current working directory.  If that directory were world writeable, then someone could add that file and it would be run when you run your test cases.

Now, it turns out that this is not a vulnerability with prove.  Because prove runs Perl in a separate process and parses the output, eliminating the resolution inside prove itself has no effect.  What this means is that the directory where you run something like prove can really matter and if you happen to be in a world writeable directory when you run it (or other Perl programs) you run the risk of including unintended code supplied by other users.  Not good.  None of the proposed fixes address the full scope of this problem either.  Note that if any directory in @INC is world-writeable, a security problem exists.  And because these can be specified in the Perl command line, this is far more of a root problem than the mere inclusion of the current working directory.

Security Considerations for System Administrators and Software Developers

All exploits of this sort can be prevented even without the recommendations being followed in the proper fixes section.  System administrators should:

1. Make sure that all directories in @INC other than the current working directory are properly secured against write access (this is a no brainer, but is worth repeating)
2. Programs such as test harnesses which execute arbitrary Perl code should ONLY be run in properly secured directories, and the only time prove should ever be run as root is when installing (as root) modules from cpan.
3. Scripts intended to be run in untrusted directories should be audited and one should ensure (and add if it is missing) the following line:  no lib '.';

Software developers should:

1. think carefully about where a script might be executed.  If it is intended to be run on directories of user-supplied files, then include the  no lib '.';  (This does not apply to test harnesses and other programs which execute arbitrary Perl programs).
2. Module maintainers should probably avoid using optional config modules to do configuration.  These optional configuration modules provide standard points of attack.  Use non-executable configuration files instead or modules which contain interfaces for a programmer to change the configuration.

What is wrong with the proposed fixes

The approach recommended by the reporter of the problem is to make modules exclude an implicit current working directory when loading optional dependencies.  This, as I will show, raises very serious separation of concerns problems and in Debian's case includes one serious bug which is not obvious from outside.  Moreover it doesn't address the problems caused by running test harnesses and the like in untrusted directories.  So one gets a *serious* problem with very little real security benefit.

If you are reading through the diff linked to above, you will note it is basically boilerplate that localizes @INC and removes the last entry if it is equal to a single dot.  This breaks base.pm badly because inheritance in Perl no longer follows @INC the way use does.  Without this patch the following are almost equivalent with the exception that the latter also runs the module's import() routine:

use base 'Myclass';

and

use Myclass;
use base 'Myclass';

But with this patch, the latter works and the former does not unless you specify -MMyclass in the Perl command line.  This occurs because someone is thinking technically about an issue without comprehending that this isn't an optional dependency in base and therefore the problem doesn't apply there.  But the problem is not quickly evident in the diff, nor is it evident when trying to fix this in this way on the module level.  It breaks the software contract badly, and does so for no benefit.

As a general rule, modules should be expected to act sanely when it comes to their own internal structure, but playing with @INC violates basic separation of concerns and a system that cannot be readily understood cannot be readily secured (which is why this is a real issue in the first place -- nobody understands all the optional dependencies of all the dependencies of every script on their system).

Recommendations for proper fixes

There are several things that Perl and Linux distros can do to provide proper fixes to this sort of problem.  These approaches do not violate the issues of separation of concerns.  The first and most important is to provide an option to globally remove '.' from @INC on a per-system basis.  This is one of the things that Debian did right in their reaction to this.  Providing tools for administrators to secure their systems is a good thing.

A second thing is that Perl already has a number of enhanced modes for dealing with security concerns and adding another would be a good idea.  In fact, this could probably be done as a pragma which would:

1. On load, check to see that directories in @INC are not world-writeable -- if they are, remove them from @INC and warn, and
2. when using lib, check to see if the directory is world-writeable and if it is die hard.

But making this a module's responsibility to care what @INC says?  That's a recipe for problems and not of solutions both security and otherwise.