Author: brian d foy

Update your YAML modules to change the default behavior to not bless data structures. But, test your code to ensure that you don’t depend on that feature or break a bunch of things through the upgrade. If you depend on the old default behavior, you can to set a variable to enable the old behavior. The original announcement has more details.

Previously I’ve written about Perl’s Storable security problem. In short, inflating a serialized object could load an arbitrary class and potentially run damaging and unintended code. YAML also has a way to inflate data into class and can have the same problems.

Here’s a program to convert a Mojo::URL object to YAML:

use v5.10;

use Mojo::URL;
use YAML;

my $url = Mojo::URL->new( 'http://www.example.com' );

say Dump( $url );

The YAML includes a the type of “object” it is, which is basically a blessed Perl reference. Unserializable things, such as filehandes or code references, will have problems. Otherwise, it’s straightforward:

--- !!perl/hash:Mojo::URL
host: www.example.com
path: !!perl/hash:Mojo::Path
  charset: UTF-8
  leading_slash: ''
  parts: []
  trailing_slash: ''
scheme: http

Now reverse that. This program starts with the YAML text (in an indented here doc) and loads it. Notice that I did not load the Mojo::URL module but I end up with a blessed reference. This uses YAML 1.29 (because a later version will do it differently):

use v5.26;

use YAML qw(Load); # This is v1.29

my $string = <<~"HERE";
	--- !!perl/hash:Mojo::URL
	host: www.example.com
	path: !!perl/hash:Mojo::Path
	  charset: UTF-8
	  leading_slash: ''
	  parts: []
	  trailing_slash: ''
	scheme: http
	HERE

my $data = Load( $string );

say 'Version: ', YAML->VERSION;
say Dumper( $data ); use Data::Dumper;

With an older YAML, I get a Mojo::URL object back, even though
I haven’t loaded that module:

Version: 1.29
$VAR1 = bless( {
                 'scheme' => 'http',
                 'host' => 'www.example.com',
                 'path' => bless( {
                                    'charset' => 'UTF-8',
                                    'leading_slash' => '',
                                    'parts' => [],
                                    'trailing_slash' => ''
                                  }, 'Mojo::Path' )
               }, 'Mojo::URL' );

The rest of the attack is the same as that for Storable. If I know ahead of time that the DESTROY method will do something, I can construct the serialization of that object to get it to act on my data. With File::Temp, I can get it to try to remove files.

This feature was reported as a bug in Debian #862373 , and then as a GitHub issue for the YAML module.

With YAML 1.30 (released on January 28, 2020) changes this behavior. Previously (back several versions) it blessed inflated objects by default. Now I get back a simple, non-blessed hash:

Version: 1.30
$VAR1 = {
          'path' => {
                      'trailing_slash' => '',
                      'leading_slash' => '',
                      'charset' => 'UTF-8',
                      'parts' => []
                    },
          'scheme' => 'http',
          'host' => 'www.example.com'
        };

I can set $YAML::LoadBlessed to a true value to get the old behavior. I don’t really want to set a global variable because I don’t know what else I’ll mess up far away in the program. I can do it in a block with local to limit the damage:

#!perl
use v5.26;

use YAML qw(Load);

my $string = <<~"HERE";
	--- !!perl/hash:Mojo::URL
	host: www.example.com
	path: !!perl/hash:Mojo::Path
	  charset: UTF-8
	  leading_slash: ''
	  parts: []
	  trailing_slash: ''
	scheme: http
	HERE

my $url = do {
	local $YAML::LoadBlessed = 1;
	Load( $string );
	};

say Dumper( $url ); use Data::Dumper;

Perl’s @INC might find code that you don’t want. That array is list of directories that use, require, and do search to find modules and libraries. By default, the last entry is ., which represents the current working directory.

J.D. Lightsey and Todd Rinaldo from cPanel (a top-shelf Perl Foundation Core Maintenance Fund sponsor) first reported the issue as CVE-2016-1238. If someone puts a module in the current working directory, they don’t have to modify @INC to load it. However, this also means that someone else might put a module in the current working directory, and, in the case that it’s not already installed somewhere else in @INC, the application loads that one. If you’re running from a shared directory, this might affect you. You don’t even need to use the module in some cases as other modules might helpfully try to use it for you, as in a plugin or Storable’s object inflation bug.

This could be a problem; It could also not be a problem. However, Perls v5.22.3 and v5.24.1 (not yet released as I write this), modify several modules and programs in the perl source tree to remove the dot from @INC.

These patches don’t actually solve the entire issue, but they are a good start. They solve a very small corner issue. It’s similar to the parts of Mastering Perl that cover taint checking and the directories in the PATH environment variable. A small fix doesn’t magically protect us from everything else that impacts the issue. We still have to think through the problem based on everything else that is going on.

Consider the patch to my module App::Cpan. I loaded some modules conditionally:

my $log4perl_loaded = eval "require Log::Log4perl; 1";

A patch to fix this @INC problem creates a subroutine that “safely” loads a module:

sub _safe_load_module {
	my $name = shift;

	local @INC = @INC;
	pop @INC if $INC[-1] eq '.';
	eval "require $name; 1";
}

This wraps my previous calls:

_safe_load_module("Log::Log4perl");

But, there are many problems with this way of doing things. If the problem is loading modules from directories writeable by other users, why would you limit yourself to looking at a single entry in @INC and only removing it if it has a hard-coded entry in only the last position?

I disagree that this is merely a program issue. It’s a risk of using a multi-user computer with every utility, application, or programming language. It’s a consequence of the way things work in these systems. This means the system provides ways, by design, to circumvent this very limited protection.

If this is truly a problem, we should examine every entry to @INC to check permissions. Most of the entries are unlikely to be writeable by other users since they are likely owned by a superuser (or other elevated-priveleges account).

Should perl track a list of trusted users? Some of the directories might be group writeable. I maintain some systems where many people are able to affect system directories. I typically have a perl group for the people allowed to administer the perl installations. Am I going to trust all of those? I think I trust them, but what if someone changed the group to add a user that I don’t trust?

What if one of the entries is a symlink? Maybe someone futzed with the filesytem so that one of the system directories points to a directory that we shouldn’t trust?

Now, suppose we have a foolproof way to check that everything about a directory is acceptable. Between the time we do that and the time we try to load the module, does that situation change? Do we need to lock everything?

So, there’s all of that. Some filesystems make it so that we can’t trust anything in @INC.

But, let’s look at the fix a bit more. There are several statements in _safe_load_module. If I can load a perl debugger here (and there are many ways to do that), I get a chance to run arbitrary code after each statement. After you change @INC I can change it right back. But, I don’t need to change it back. I can append an entry so . is not the last one. Or, what if I had already appended to @INC in such a way that there were two entries for the current directory? Did you know that you can have a code reference in @INC?

I don’t even have to trick you into loading a module you don’t know about. If you’re using CPAN, you’re taking far more risk than loading modules from a writeable directory. How many of you actually review the code in the modules you use? I like Mojolicious, I trust it, and I’ve read a lot of the code, but I haven’t read all of it. When I upgrade, I don’t review the code changes.

What if they decide to play around with @INC? Every time I install a module from CPAN I take the slight risk that someone has tricked me into installing malicious code, the slightly greater risk of installing exploitable code, and the certainty of installing buggy code.

There are some many ways to get to the same effect that deciding to not trust one particular string in one particular position ignores the actual problem. Maybe someone feels safer, but that plays into the hands of the malicious people who know what they are doing and realize this patch won’t stop them.

The removal of . is a small, but good step to the much larger problem. We still need to consider what we allow.

Perl’s @INC might find code that you don’t want. That array is list of directories that use, require, and do search to find modules and libraries. By default, the last entry has been ., which represents the current working directory. That’s not a real directory; it’s a pointer to a directory you’ll discover later. There’s a fix for one consequence of this problem, but there are still issues of trust. That’s security—there are always more problems.

Continue reading “Perl v5.26 removes . from @INC, but don’t think you’re safe!”