Skip to content
Browse files

Plack::App::File: Fix a security issue by not pruning trailing slashes

Before this Plack::App::File would prune trailing slashes via its split
invocation. I.e. it would think this:

    $ perl -MData::Dumper -wle 'print Dumper [split /[\\\/]/, shift]' a/file.txt
    $VAR1 = [

Was the same as:

    $ perl -MData::Dumper -wle 'print Dumper [split /[\\\/]/, shift]' a/file.txt///
    $VAR1 = [

This can. turn into a nasty code exposure issue if you e.g. have an app
that basically does this:

    1. I'd do a regex /\z/ on a file to see if it was a text file
    2. If so, do magic to generate text file via perl
    3. Else it's not a /\z/ file, so it must be some other static
       file with a different extension
    4. Serve it up with Plack::Middleware::Static

This is also not how other webservers or Unix utilities work:

    $ touch /tmp/foo.txt
    $ file /tmp/foo.txt
    /tmp/foo.txt: empty
    $ file /tmp/foo.txt/
    /tmp/foo.txt/: ERROR: cannot open `/tmp/foo.txt/' (Not a directory)

This resolves issue plack#405 that I filed around 9 months ago. I was
previously working around it in my own code by doing:

        # Let's see if someone's trying to be evil by
        # requesting e.g. /index.html/ instead of
        # /index.html. We don't want to fall through
        # and just serve up the raw content.
        my $plack_app_file = Plack::App::File->new({ root => PLACK_WEBSERVER_DOCUMENT_ROOT() });
        my ($file) = $plack_app_file->locate_file($env);
        if (
            # We'll get a reference if it's a full
            # Plack response. I.e. a 404 or whatever.
            ref $file ne 'ARRAY'
            # WTF once we canonicalize the file and it
            # looks like a Mason handled path let's
            # not accept it, because we don't want to
            # serve up the raw unprocessed Mason page
            # via this hack.
            $file =~ $mason_handles_this_path_rx
        ) {
            TELL "Middleware::Static: Path <$path> request, doesn't match <$mason_handles_this_path_rx>, but actually resolves to it via resolved file <$file>" if DEBUG;
            # Tells our app to just serve up a
            # 400. Apache would do a 404 but I think
            # these requests are bad, so say so.
            $env->{$magic_marker_to_return_400} = 1;
  • Loading branch information...
avar committed Feb 7, 2014
1 parent 9c8a5bc commit bc1731dbb53850c380875ad683cd87c8ec99eee3
Showing with 31 additions and 1 deletion.
  1. +11 −0 Changes
  2. +1 −1 lib/Plack/App/
  3. +19 −0 t/Plack-Middleware/file.t
11 Changes
@@ -1,6 +1,17 @@
Go to for the roadmap and known issues.

- Plack::App::File would previously strip trailing slashes off
provided paths.

This in combination with the common pattern of dynamically
generating some files in a tree and serving the rest up with
Plack::Middleware::Static could allow an attacker to bypass
a whitelist of generated files by just requesting
/file.disallowed/ instead of /file.disallowed, provided that
Plack::Middleware::Static was used for all paths except
those matching /\.disallowed$/

1.0030 2013-11-23 08:54:01 CET
@@ -44,7 +44,7 @@ sub locate_file {

my $docroot = $self->root || ".";
my @path = split /[\\\/]/, $path;
my @path = split /[\\\/]/, $path, -1; # -1 *MUST* be here to avoid security issues!
if (@path) {
shift @path if $path[0] eq '';
} else {
@@ -3,6 +3,7 @@ use Plack::Test;
use Test::More;
use HTTP::Request::Common;
use Plack::App::File;
use FindBin qw($Bin);

my $app = Plack::App::File->new(file => 'Changes');

@@ -35,6 +36,24 @@ test_psgi $app_content_type, sub {
is $res->code, 200;

my $app_secure = Plack::App::File->new(root => $Bin);

test_psgi $app_secure, sub {
my $cb = shift;

my $res = $cb->(GET "/file.t");
is $res->code, 200;
like $res->content, qr/We will find for this literal string/;

my $res = $cb->(GET "/../Plack-Middleware/file.t");
is $res->code, 403;
is $res->content, 'forbidden';

for my $i (1..100) {
$res = $cb->(GET "/file.t" . ("/" x $i));
is $res->code, 404;
is $res->content, 'not found';


0 comments on commit bc1731d

Please sign in to comment.
You can’t perform that action at this time.