Yet another taint mode reminder

Schwern just posted "How (not) To Load a Module..." that goes into great depth about the security risk in loading modules.

The (not) funny thing is that none of what he's saying is a risk would be one when running in taint mode.

Consider "/tmp/foo.pm" with this:

package foo;
print "Loaded foo\n";
1;

Then consider this example of how Module::Load does something "unexpected":

$ perl -MModule::Load=load -wE 'my $file=shift; load $file' ::tmp::foo
Loaded foo

(The "threat" is that given an arbitrary module name to load, it will gladly load outside @INC.)

What if that was run under taint mode, instead?

$ perl -MModule::Load=load -wTE 'my $file=shift; load $file' ::tmp::foo
Insecure dependency in require while running with -T switch at /home/david/perl5/perlbrew/perls/perl-5.14.0/lib/5.14.0/Module/Load.pm line 27.
Insecure dependency in require while running with -T switch at /home/david/perl5/perlbrew/perls/perl-5.14.0/lib/5.14.0/Module/Load.pm line 27.

I'm not sure why that message is printed twice, but that was still a fatal error and foo.pm didn't load.

The moral of the story: if you incorporate arbitrary user input into your execution path, use taint mode and validate the input to make sure it's something safe.

This entry was posted in perl programming and tagged , , , . Bookmark the permalink. Both comments and trackbacks are currently closed.

11 Comments

  1. Posted October 4, 2011 at 1:00 pm | Permalink

    This would be great if not for the fact that taint mode is a buggy mess. In theory, Perl should work the same whether tainting is on or off, except for the things that taint is documented to do.

    In practice, taint mode is buggy as hell, and in my experience has caused random failures with regexes, XS code, slowdowns, and other fun.

    I don't think taint mode is really well maintained in the core. One thing I think would help would be to arrange to run (almost) all of the existing tests under taint mode. In theory, they should pass. In practice, I suspect this would uncover a number of bugs.

    • Paul Harvey
      Posted November 28, 2011 at 9:14 pm | Permalink

      I'm still trying to appreciate your comment here. "buggy mess"? not "well maintained in core"? Are we supposed to ignore everything in man perlsec?

      T/Foswiki have been running for many years with taint mode in production. I've only been using Foswiki for a couple of years, but I've always been able to fix every bug that I wanted to squash, without blaming perl core...

      Can you be more specific about how taint mode causes problems?

  2. Posted October 4, 2011 at 1:05 pm | Permalink

    Price of security, perhaps. :-)

    Moral is still not to use unvalidated user input. Should module loading tools do the validating? Maybe, but then they are less general-use tools. Should they offer a user callback to validate in lieu of a reasonable default? Possibly. But putting the validation burden on libraries far from user input seems to put the complexity in the wrong place.

  3. Posted October 4, 2011 at 7:26 pm | Permalink

    - Not all code can be run under taint mode.
    - A principle for writing a secure piece of code is to not trust input.
    - Module loading *should* be secure.

    • Posted October 4, 2011 at 7:51 pm | Permalink

      Perhaps it "should" but the reality is that it is not. Perl's require() will load any arbitrary path you give to it. That is inherently insecure. Any library that wants to mimic that behavior is inherently insecure.

      I am concerned about the "insecurity" of module loading to the same degree that I am concerned about system(), exec() and eval() -- which is to say that I know that these functions execute arbitrary code and I don't pass user-generated input to them without validation.

      The job of a module-loader is to load modules, not validate input (beyond the basic sanity checks needed to do its job). Having the system or low level libraries make assumptions about what is valid or invalid makes them brittle. What if I have a test module that creates a .pm file in a temporary directory and then I want to load it (with an absolute path)? Should it not load because it's not in @INC? That would be an absurd loss of functionality in the name of "security"

  4. bingos
    Posted October 5, 2011 at 9:41 am | Permalink

    Module::Load has been fixed for this particular problem (version 0.22)

  5. brian d foy
    Posted October 5, 2011 at 12:17 pm | Permalink

    Taint mode doesn't do anything for you by itself. It's solely exists to tell you when you have to untaint data. However, it doesn't care how you untaint data and once untainted, doesn't care what you do with that data. Tainting is not a security tool. It's a development tool, and it's easily bypassed. I have most of a chapter on this in Mastering Perl. Taint mode helps people who already know what they are doing. For people who don't know what they are doing, it's most likely going to do nothing for them as they untaint with /(.*)/.

    • Posted October 5, 2011 at 1:27 pm | Permalink

      Absolutely. It flags that the developer is doing something insecure -- like taking user input and passing it to a module loader. If a user decides to untaint with /(.*)/ it's no worse than not bothering with taint mode, but at least the developer is being explicit in that choice.

      Ultimately -- it's a computer program. If one programs it to do insecure things, it will follow along and do them.

  6. Sterling Hanenkamp
    Posted October 12, 2011 at 1:14 am | Permalink

    Repeating some of what has already been commented upon, but part of what Schwern pointed out is that even the modules that purport to validate your runtime module loading don't actually do the input validation well enough because doing so is complicated. So even if you use taint mode you have to be very careful about how you craft your validation patterns. Better to just stick to one of these modules at getbit right once, but get the problems that Schwern found in some of them cleaned up.

    • Posted October 12, 2011 at 10:44 am | Permalink

      As far as I can tell, this whole episode started because of a CVE filed on a module (which is not a module loader) -- let's call it Module A -- that takes a class name for an implementation subclass and was loading it with string eval without sufficient validation. The actual security hole was another module -- call it Module B -- that was taking arbitrary user input for which implementation should be used to process certain data and handing it off to Module A, also without validation.

      That's two modules that didn't bother to validate that a class name was actually a class name. Schwern is totally right that module loading should be done better to avoid these kinds of security holes. But one rarely knows when some module in your dependency chain might do a string eval (or something else unsafe) for some particular reason, so it's good practice to validate your input when you get it from your user rather than hoping (praying?) that all the modules that consume user data are doing the right thing.

  7. Paul Harvey
    Posted November 28, 2011 at 6:06 am | Permalink

    Some of the Foswiki guys are working on re-implementing Foswiki::Engine to use PSGI - great news. But I was terribly surprised to learn that taint mode for Plack apps isn't recommended in production.

    Surely if Foswiki (actually TWiki, where taint mode was introduced before the fork) can do it, the mighty perlmonks can do it too?

    It might not make software safe; it might even give a false sense of security - but I just can't forget that I've caught a lot of bugs thanks to taint mode. Especially in dodgy plugins, or CPAN stuff Eg. Bio::* modules that were never written with a CGI environment in mind.

    Taint mode prompted take-up of the Foswiki::Sandbox API (uses a printf-esque, typed-argument template thing to build a properly quoted command string) for replacing system calls. This isn't by itself necessarily much safer than system() but like taint mode, provides plugin authors which don't think about security (or mightn't even be programming in their $dayjob) a pattern which hopefully prompts them to.

    As for taint mode causing bugs in perl & core modules, that's highly surprising as well - Foswiki is *nothing but* a nightmarish spaghetti of regexes - but I'm yet to hear of us hitting a perl regex bug attributed to taint mode...

    Anyway, I do believe we've closed a lot of gaping holes, and apart from that ended up with a more robust codebase thanks to habits introduced from making taint mode mandatory.

© 2009-2014 David Golden All Rights Reserved