Saturday, July 30, 2016

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.

7 comments:

  1. I have decided to start a full disclosure series on this type of vulnerability in Perl because as I have come to understand it, it is clear to me that the current stop-gap proposed patches only obscure the problem and don't actually help sysadmins secure their systems.

    Topics covered will include the dangers of test harnesses, scripts that process user-submitted data (how to audit, secure these, and fix them), best practices for writing system admin perl scripts for secure systems, and tips for avoiding making this topic worse for software developers.

    ReplyDelete
  2. Hi,

    Nice write up, thank you. I'm trying to make myself oriented in the situation.

    The problem is that attacker may force under some circumstances perl program to
    load his code. If it is regular user - so what, he can run his code directly,
    no need to bother abusing existing perl program. If it is regular user running
    script which is suid, uses RBAC or otherwise 'gains' privileges, it may be
    problem. If you are root and you ran random perl script you certainly don't
    want the script to load random modules from current directory, /tmp or so.

    How the attack can be performed? You either use the dot in @INC from default
    perl configuration, or you try to force your directory onto @INC before the
    script loads it's modules.

    You mention that @INC can be modified from command line. Sure, it can

    perl -I . -MData::Dumper a.pl

    and if a.pl is using Data::Dumper and you created Data/Dumper.pm you just got
    it loaded/executed. But is that a problem? If it's regular script, ran by
    regular user - so what, he can run the commands directly. If it's suid script
    and you try to call it via

    perl -I. /usr/bin/suid.pl
    or
    sudo perl -I. /usr/bin/suid.pl

    It won't get increased privileges, because you execute perl binary, not the script.

    If you are root, you surely don't run your commands via 'perl -I.'.

    So the possibility that you can alter @INC from command line is not a security issue.


    Worse is if you try to abuse the default config having '.' in @INC.

    If you are regular user, again 'so what?'. You just executed a code you could
    be running directly anyway. If that code leads to security breach you wouldn't
    need perl to do that in first place.

    If it is a suid binary, my opinion is that it should be tainted. If it is not,
    you get what you ask for.

    If you are root, and you run random command which happens to be implemented in
    perl, it should not consider random directories for module load.

    I like the sitecustomize.pl way. I like to have the possibility to remove '.'
    in @INC from perl. But it will break so many things it seems.

    My disagreement is with checking whether the dir is world writable. That is not
    easy task if you take ACLs into account.


    Cheers
    __
    Vlad

    ReplyDelete
    Replies
    1. This is a more complex and hidden problem than that (please see my more recent post on use lib '.' for what you are describing).

      The problem here is that a module typically requires other modules and they require other modules.... And some of these modules in the dependency tree optionally require some modules either for additional functionality or for configuration.

      With the current working directory at the end of the path, those optional modules get eventually resolved to the current working directory because they are not found elsewhere on the system. In a non-trivial program, knowing what optional dependencies there are is a non-trivial problem. And so attackers have the opportunity to try to put various obscure .pm's that people don't recognize as harmful in working directories and those get executed.

      That's a very different problem than the one you describe becuase -I. puts the current working directory at the front of the include path, and that allows an attacker to exploit modules that are otherwise on the system.

      Delete
    2. It is worth noting though that both are garden path problems. What a program *appears* to be doing from reading its code and what it *is* doing are not necessarily the same thing. But they pose very different security challenges.

      This CVE's challenge comes from the complexity of knowing what optional dependencies are found in a script or program. This leads to an obscurity challenge. your un json_pp.pm in a directory that includes an EncodeConfig.pm? Do you expect that to be run? Depending on the script you just ran, maybe it is.

      For the -I. question though you have a different question in that if you run a script that uses a module that uses Cwd.pm in a directory that includes a Cwd.pm. that module can load the correct module while injecting arbitrary code into your program.

      In my view both are serious issues. But they are different issues.

      Delete
    3. It is worth noting though that both are garden path problems. What a program *appears* to be doing from reading its code and what it *is* doing are not necessarily the same thing. But they pose very different security challenges.

      This CVE's challenge comes from the complexity of knowing what optional dependencies are found in a script or program. This leads to an obscurity challenge. your un json_pp.pm in a directory that includes an EncodeConfig.pm? Do you expect that to be run? Depending on the script you just ran, maybe it is.

      For the -I. question though you have a different question in that if you run a script that uses a module that uses Cwd.pm in a directory that includes a Cwd.pm. that module can load the correct module while injecting arbitrary code into your program.

      In my view both are serious issues. But they are different issues.

      Delete
  3. Hi,

    I tried to explore all the possibilities how malicious *.pm can be executed. Yes, those are two different attacks. And having optional dependency will have the same outcome be it in the script or be it one of the libraries used during it's run.

    What didn't occur to me at first was that attacker does not need to be targeting root user - he can target any other user too. He won't gain privileges, but hey, ssh keys are surely worth something too.

    Thank you
    __
    Vlad

    ReplyDelete
    Replies
    1. Or other real-world valuable information or actions. One reason we take security so seriously regarding LedgerSMB is that these sorts of things could be of great interest for those involved in embezzlement and other malfeasance.

      Delete