[Rt-commit] rt branch, 4.0/dotfile-downloads, created. rt-4.0.19-62-ga51e3df

[Alex Vandiver]

The branch, 4.0/dotfile-downloads has been created
        at  a51e3df7654424572923a2149611508e03e386a6 (commit)

- Log -----------------------------------------------------------------
commit a51e3df7654424572923a2149611508e03e386a6
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Mon Mar 31 19:32:25 2014 -0400

    Allow downloading dotfiles
    
    Our directory-traversal code overzealously rejected requests for
    /Ticket/Attachment/100/200/.bashrc and the like, making the "download"
    links for uploaded dotfiles appear to return a blank page.
    
    Allow /.something, but reject /./, /../, ../, /., /.., and the like.
    
    Fixes issues #29700.

diff --git a/lib/RT/Interface/Web/Handler.pm b/lib/RT/Interface/Web/Handler.pm
index 37031b1..07e7707 100644
--- a/lib/RT/Interface/Web/Handler.pm
+++ b/lib/RT/Interface/Web/Handler.pm
@@ -278,7 +278,7 @@ sub PSGIApp {
         # CGI.pm normalizes .. out of paths so when you requested
         # /NoAuth/../Ticket/Display.html we saw Ticket/Display.html
         # PSGI doesn't normalize .. so we have to deal ourselves.
-        if ( $req->path_info =~ m{/\.} ) {
+        if ( $req->path_info =~ m{(^|/)\.\.?(/|$)} ) {
             $RT::Logger->crit("Invalid request for ".$req->path_info." aborting");
             my $res = Plack::Response->new(400);
             return $self->_psgi_response_cb($res->finalize,sub { $self->CleanupRequest });
diff --git a/t/web/path-traversal.t b/t/web/path-traversal.t
index 5d5c954..01302e6 100644
--- a/t/web/path-traversal.t
+++ b/t/web/path-traversal.t
@@ -1,9 +1,10 @@
 use strict;
 use warnings;
 
-use RT::Test tests => 22;
+use RT::Test tests => undef;
 
 my ($baseurl, $agent) = RT::Test->started_ok;
+ok($agent->login);
 
 $agent->get("$baseurl/NoAuth/../Elements/HeaderJavascript");
 is($agent->status, 400);
@@ -31,6 +32,12 @@ SKIP: {
     $agent->warning_like(qr/Invalid request.*aborting/,);
 };
 
+# Do not reject a simple /. in the URL, for downloading uploaded
+# dotfiles, for example.
+$agent->get("$baseurl/Ticket/Attachment/28/9/.bashrc");
+is($agent->status, 200); # Even for a file not found, we return 200
+$agent->content_contains("Bad attachment id");
+
 # do not reject these URLs, even though they contain /. outside the path
 $agent->get("$baseurl/index.html?ignored=%2F%2E");
 is($agent->status, 200);
@@ -44,3 +51,5 @@ is($agent->status, 200);
 $agent->get("$baseurl/index.html#/.");
 is($agent->status, 200);
 
+undef $agent;
+done_testing;

-----------------------------------------------------------------------