The Storable security problem

Recently, people have moved to close, or at least document, a security issue with Storable. This core module serializes and deserializes Perl data structures, and, as in many places in Perl, tries to be more helpful than we really want. In Mastering Perl, I talk about lightweight persistence in Chapter 14; Storable is a big part of that chapter.

There are two major problems. Someone can force Storable to load arbitrary modules, and someone can possibly run unexpected code.

Mal-stored hashes

If you look in Storable.xs, you’ll find a couple of instances of a call to load_module. Depending on what you’re trying to deserialize, Storable might load a module without you explicitly asking for it.

The first comes from the design of Storable itself. My Perl module can define serialization hooks that replace the default behavior with my own. I can serialize the object myself and give Storable the octets it should store. Along with that, I can take the octets from Storable and recreate the object myself. Perhaps I want to reconnect to a resource as I rehydrate the object.

Storable notes the presence of a hook with a flag set in the serialization. As it deserializes and notices that flag, it loads the module to look for the corresponding STORABLE_thaw method.

The same thing happens for classes that overload operators. Storable sets a flag and when it notices that flag, it loads the module.

It doesn’t really matter that a module actually defines hooks or uses overload. The only thing that matters is that the serialized data sets those flags. If I store my data through the approved interface, bugs aside, I should be fine. If I want to trick Storable, though, I can make my own file and set whatever bits I like. If I can get you to load a module, I’m one step closer to taking over your program.

Mal-stored hashes

I can also construct a special hash serialization that tricks perl into running a method. If I use the approved interface to serialize a hash, I know that a key is unique and will only appear in the serialization once.

Again, I can muck with the serialization myself to construct something that Storable would not make itself. I can make a Storable string that repeats a key in a hash.

use v5.14;
use Storable qw(freeze thaw);
use Data::Printer;

say "Storable version ", Storable->VERSION;

package Foo {
	sub DESTROY { say "DESTROY while exiting for ${$_[0]}" };
	}

my $frozen = do {
	local *Foo::DESTROY = sub { 
		say "DESTROY while leaving scope for ${$_[0]}" };
	my $data = {
		'key1' => bless( \ (my $n = 'abc'), 'Foo' ),
		'key2' => bless( \ (my $m = '123'), 'Foo' ),
		};
	say "Saving...";
	freeze( $data ) =~ s/key2/key1/r;
	};


my $unfrozen = do {
	say "Retrieving...";
	local *Foo::DESTROY = sub { 
		say "DESTROY while inflating for ${$_[0]}" };
	thaw( $frozen );
	};

say "Done retrieving, showing hash...";

p $unfrozen;

say "Exiting next...";

In the first do black, I create a hash with key1 and key2, both which point to scalar references blessed into Foo. I freeze that and immediately change the serialization to replace the literal key2 with key1. The munged version ends up in $frozen. If you haven’t fallen in love with the /r yet, see Use the /r substitution flag to work on a copy.

When I want to thaw that string, I create a local version of DESTROY to watch what happens. In the output I see that I handle one instance of key1 then destroys it when it handles the next one. At the end I have a single key in the hash:

Storable version 2.39
Saving...
DESTROY while leaving scope for 123
DESTROY while leaving scope for abc
Retrieving...
DESTROY while inflating for 123
Done retrieving, showing hash...
\ {
    key1   Foo  {
        public methods (1) : DESTROY
        private methods (0)
        internals: "abc"
    }
}
Exiting next...
DESTROY while exiting for abc

If I haven’t already forced Storable to load a module, I might be able to use the DESTROY method from a class that’s I know is already loaded. One candidate is the core module CGI.pm which includes the CGITempFile class which tries to unlink a file when it cleans up an object:

sub DESTROY {
    my($self) = @_;
    $$self =~ m!^([a-zA-Z0-9_ \'\":/.\$\\~-]+)$! || return;
    my $safe = $1;             # untaint operation
    unlink $safe;              # get rid of the file
}

Although this method untaints the filename, remembering that taint checking is not a prophylactic; it’s a development tool. Untainting whatever I put in $$self isn’t going to stop me from deleting a file, including, perhaps, the Storable file I used to deliver my malicious payload.

Alternatives

There’s no fix for Storable, although cPanel had a patch that disabled the problematic features. It’s still fundamentally flawed. This problem isn’t limited to Storable either. Any deserializer which loads and runs code for you can do the same things. At YAPC::EU, Jonathan Stowe showed this with Data::DumpXML::Parser in his The Perils of Persistence talk.

Storable isn’t designed for the exchange of untrusted data. You might be able to use JSON, which is designed for untrusted data. There’s a new (cross platform) serializer, Sereal, that has Perl bindings in Sereal::Encoder and Sereal::Decoder.