[libplack-perl] 01/02: Include bc1731d from 1.0031 release to fix CVE-2014-5269.

gregor herrmann gregoa at debian.org
Wed Oct 8 21:06:12 UTC 2014


This is an automated email from the git hooks/post-receive script.

gregoa pushed a commit to branch wheezy
in repository libplack-perl.

commit 0e191d221ece1c8e48d44e624ac6448b889e9702
Author: gregor herrmann <gregoa at debian.org>
Date:   Wed Oct 8 23:00:51 2014 +0200

    Include bc1731d from 1.0031 release to fix CVE-2014-5269.
    
    Plack::App::File would previously strip trailing slashes off provided paths.
    
    This could under specific circumstances lead to the unintended delivery of
    files. For details see the pull request message preserved in
    debian/patches/01-fix-CVE-2014-5269.patch.
---
 debian/patches/01-fix-CVE-2014-5269.patch | 130 ++++++++++++++++++++++++++++++
 debian/patches/series                     |   1 +
 2 files changed, 131 insertions(+)

diff --git a/debian/patches/01-fix-CVE-2014-5269.patch b/debian/patches/01-fix-CVE-2014-5269.patch
new file mode 100644
index 0000000..9209e8b
--- /dev/null
+++ b/debian/patches/01-fix-CVE-2014-5269.patch
@@ -0,0 +1,130 @@
+From b46ccc15188d979eb062582d53de68a7a9ddeab2 Mon Sep 17 00:00:00 2001
+From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?=
+ <avarab at gmail.com>
+Date: Fri, 7 Feb 2014 10:13:45 +0000
+Subject: [PATCH] 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 = [
+              'a',
+              'file.txt'
+            ];
+
+Was the same as:
+
+    $ perl -MData::Dumper -wle 'print Dumper [split /[\\\/]/, shift]' a/file.txt///
+    $VAR1 = [
+              'a',
+              'file.txt'
+            ];
+
+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 /.txt.pl\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 /.txt.pl\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 #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'
+            and
+            # 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;
+            return;
+        }
+    }
+
+---
+ lib/Plack/App/File.pm     |  2 +-
+ t/Plack-Middleware/file.t | 19 +++++++++++++++++++
+ 2 files changed, 20 insertions(+), 1 deletion(-)
+
+diff --git a/lib/Plack/App/File.pm b/lib/Plack/App/File.pm
+index b437237..f524351 100644
+--- a/lib/Plack/App/File.pm
++++ b/lib/Plack/App/File.pm
+@@ -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 {
+diff --git a/t/Plack-Middleware/file.t b/t/Plack-Middleware/file.t
+index 41753fa..0f2ec0b 100644
+--- a/t/Plack-Middleware/file.t
++++ b/t/Plack-Middleware/file.t
+@@ -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 => 'README');
+ 
+@@ -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';
++    }
++};
+ 
+ done_testing;
+-- 
+2.1.1
+
diff --git a/debian/patches/series b/debian/patches/series
new file mode 100644
index 0000000..694081c
--- /dev/null
+++ b/debian/patches/series
@@ -0,0 +1 @@
+01-fix-CVE-2014-5269.patch

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-perl/packages/libplack-perl.git



More information about the Pkg-perl-cvs-commits mailing list