October 2011 Archives

How (not) To Load a Module or Bad Interfaces Make Good People Do Bad Things

tl;dr version: The design of require makes it all but impossible to use in a secure and correct fashion. To fix this, Perl needs two new ops: one which will only load files, and one which will only load modules. Both would only load from @INC.

The more I look into the problem, the more I'm convinced that there is no good way to load a module from a variable in Perl. None of the existing techniques or modules fully solve the problem. They all have security holes or limitations. This is kind of embarrassing, it's an easy thing and it should be easy. My investigation into how many ways the simple act of loading a module can go wrong has lead me to believe that the solution is a new op which just loads modules.

For those of you wondering, near as I can tell this is how you correctly and securely load a module from a variable...

sub require_module {
    my $module = shift;

    # Is it defined?
    die unless defined $module;

    # Is the caller using utf8?
    require utf8;
    my $with_utf8 = (caller(0))[8] & $utf8::hint_bits;

    # Are Unicode package names ok?
    my $check = $with_utf8 ? qr{\A [[:alpha:]_] [[:word:]]*    (?: :: [[:word:]]+ )* \z}x
                           : qr{\A [A-Z_a-z]    [0-9A-Z_a-z]*  (?: :: [0-9A-Z_a-z]+  )* \z}x;

    # Is it a syntactically valid module name?
    die unless $module =~ $check;

    # Transform to a pm file path
    my $file = $module;
    $file .= ".pm";
    $file =~ s{::}{/}g;

    # What were we doing again?
    return require $file;
}

Isn't that EASY?! Nothing I've looked at gets this all correct. And please don't cut and paste that into your code, fix one of the module loading modules instead.

This all started when I mentioned on StackOverflow that having your compiler and your exception handler share the same function is a security hole by way of a bad interface. It encourages people to use them interchangeably without really thinking about the consequences. The common example I came up with was eval "require $module" which happens a lot.

The real fun came when I tried to fix it.

See, I found a security hole in a major module (details withheld because I'd rather not point it out too conspicuously before it gets updated) which allows arbitrary code execution. Whoops. Yes, it did filter $module... or it tried to anyway. And yes, it was written by a very competent programmer. Loading a module from a variable is hard. It should be easy. Easy things should be easy.

My first solution was to replace it with eval { require $module }. Yeah, it's wrong. I was in a rush. My superhuman strength was needed to lift boxes, some of which were solidly packed with metal bars. It may have been a bank heist, the details are hazy. Funny enough, the module's tests did not catch the mistake (they do now).

If you know why it's wrong, skip ahead to the next tl;dr. The rest of you sit down and learn where this whole problem comes from.

require is really two functions and that's where the whole problem comes from. Are you seeing a theme? If you pass require a variable, like require $module, it will consider that to be a file path and try to load it, appending it to each entry in @INC assuming the path is not absolute (that becomes a concern later). If you pass require a bareword, like require Module::Name, it will consider that to be a module, validate it (probably the Perl compiler validates it), transform it into a .pm file path and then do what require $scalar does.

Got that? Confused? It's hard to keep straight, and that's half the problem. Let's make it clear(er):

require Bareword::Name

  • Bareword::Name treated as a module name.
  • Checked it is valid module name syntax.
  • Transformed into a relative .pm file (ex. Bareword/Name.pm)
  • Searched through @INC.

require $file

  • $file treated as a file path.
  • No checks are done.
  • Absolute paths and paths starting with "./" are simply loaded.
  • Other paths search through @INC.

require Bareword::Name is safe in that it will load a .pm in your @INC and nothing else. If the content of @INC can be trusted, you're good. (If it can't, you shouldn't be wasting time reading overly verbose security blog posts. Fix that shit now.)

require $file is not safe. It does no validation. It can be used to load any valid Perl on the filesystem. Got that? If an attacker injects a file into /tmp an unprotected require $file might let them execute it with elevated privileges. This is bad. It gets worse.

Trouble is, require Bareword lets you load from a bareword and not a variable. This is how Perl knows you're asking for a module and not a file. Rather than have two clearly defined functions we have clever syntax. Yaaay... guh.

The user is forced into two ways around this inflexibility, both unpalatable. Either you write eval "require $module" to trick Perl into thinking $module is a bareword, or you take it upon yourself to transform $module into a pm file path and use require $file. Both are security holes.

Welcome back tl;dr readers! Wipe the drool off the side of your face, we're not done.

Some of you are thinking, "I'm not stupid. I can write eval "require $module"! All I have to do is validate $module or make sure it comes from a trusted source!" Maybe.

Once upon a time my friend was showing off this new Glock. He pointed out the lack of a conventional safety, instead there's a number of very clever things to ensure the gun will not fire unless the trigger is deliberately pulled. On the other hand, a loaded Glock will always fire if the trigger is deliberately pulled. This is a great feature if you are, say, a trained police officer. If you're a civilian, this is a great way to shoot you or your house guest in the foot. My friend was so confident in the safety of his gun he bragged he could hammer nails with it.

Just because you can hammer nails with a gun doesn't mean you should build a house with it. Just because you think you can make eval "require $module" safe doesn't mean it is. Or that everyone else will. The best security is the security you don't have to be careful about. You always have to be careful with eval STRING, every single time.

You will get lazy and not validate. Or a clever attacker will figure a way around your validation. Or your validation will reject valid module names. Or code which previously only took safe $module is later changed to take unsafe user input. I've seen all of this. This is not the path to security.

So what about require $file? Because it's so easy to jump the @INC rails that starts out insecure by design. Here's how...

Let's say an attacker has found a way to get files onto your filesystem, doesn't matter what owner. This is already a security hole, but they don't have any way to execute the file. A cracker will now find a way to escalate their privilege stacking one minor security hole onto another to make a great big one. An unprotected require $file is all they need to execute it, because it will load any absolute path. Already require is a security risk because you have to jump through more hoops to make it stick to @INC.

Filter for absolute paths and you're good? No, also paths starting with ./. And ../ because those will jump out of @INC too. Don't forget to check for .\ and .\ on Windows! Writing that down? Good. Save space, there's more.

This is about loading a module, not any old file. Usually people will first transform the module into a file path. That doesn't seem so hard, right?

my $file = $module;
$file .= ".pm";
$file =~ s{::}{/}g;
require $file;

Done? No. It's not even totally valid, it doesn't handle the ' namespace separator, but only Klingons and hippies care about that. No, it's worse. It's a security hole and one that is all over our Perl code.

Remember how require $file can be made to jump the rails and load any file? You'd think the module transformation would prevent that, but no. Consider the "module" /tmp/LOL/PWND which the code above happily loads a /tmp/LOL/PWND.pm. Ok, maybe your code has some validation and only lets things that look like module names in. If the validation isn't well written it might allow ::tmp::LOL::PWND.

The combination of a salad of insecurities makes loading a module almost impossible to properly secure unless you are very knowledgeable and very careful.

That's fine, in situations like this the smart and safe thing is to use a module! Then lots of people who think about this stuff can get it right. Right? Surely a module dedicated to the one act of loading a module has thought this through? Right? Please?

Nope.

perl5i gets it wrong. $module->require does not validate $module and so is vulnerable to the ::tmp::LOL::PWND trick.

Module::Load will jump out of @INC just like require does.

Class::Load comes close. I haven't been able to make it do anything insecure, but it tells me things which are not valid module names, like "123", are and things which are valid module names, like "Mégå::Mödulé" are not valid. Also it has a rather long dependency chain just to safely load a module.

UNIVERSAL::require works great is vulnerable to shenanigans. By making require a class method call, it uses Perl's own idea of what a valid package name is. Unfortunately, what the Perl parser considers a package name and what the Perl runtime considers a package name is different. The runtime will accept all sorts of junk for the purposes of a class method call, like subdir/../../../../../tmp/LOL/PWND. For fuck's sake.

Even if a module is made convenient, correct and secure, people will still avoid adding a CPAN dependency for something as "trivial" as loading a module. The CPAN modules should be fixed to be sure, but they are not the long term solution.

The design of require makes it almost impossible to secure. Those modules went past a lot of eyeballs, including mine, and they still don't work right. While it is good to patch those modules, what is needed is a better require built right into Perl. More precisely, two.

The first, let's call it require_module, loads modules. It only loads modules in @INC and does nothing else. By doing this inside Perl it can perfectly validate the module name avoiding both validation mistakes and security holes.

The second, let's call it require_file, loads files. It only loads files from inside @INC. Absolute paths and those which try to updir are rejected. This allows a programmer to be sure what Perl code can be loaded is at least coming from secured locations.

require_module is easy, it can be carved out of the existing require code without much trouble (uhh... as far as writing new keywords in Perl goes). require_file is made difficult because Perl lacks the built in file path operations. Detecting an absolute path is pretty straightforward, but detecting one that's trying to updir is not. That requires a complete file path parsing library... in C.

Fortunately, only require_module is needed to solve our problem. With an in core way to securely and correctly load a module from a variable, the impulse to use any other hack fades away with older versions of Perl. A CPAN module can encapsulate the decision to use the built-in or a pure Perl work around depending on the version of Perl.

I'm writing up the tests and docs now. I'll be doing the work in a branch on Github called feature/split_require if you'd like to contribute. It especially needs someone who can write the actual C code using something better than my rusty chainsaw.

About Michael G Schwern

user-pic Ya know, the guy who tells you to write tests and not use MakeMaker.