Skip to content

Commit 7c3f9a2

Browse files
committed
Protect static file routes from directory traversal attacks.
Currently using a URL like `http://baseURL/../../../../../../../../../../../../../../../../etc/passwd` returns the file `/etc/passwd`. That of course is a security vulnerability that should not happen. This was recently discovered and fixed for webwork2, and basically the same solution is used to fix it here. Below is a minimal script that can be used to test this: ```perl use Mojo::Base -strict; use Mojo::UserAgent; my $server = 'http://localhost:3001'; my $ua = Mojo::UserAgent->new; my $res = $ua->get("$server/../../../../../../../../../../../../../../../../etc/passwd")->result; if ($res->is_success) { say 'success'; say $res->body } elsif ($res->is_error) { say 'error'; say $res->message } elsif ($res->code == 301) { say '301'; say $res->headers->location } else { say 'Whatever...' } Mojo::IOLoop->start unless Mojo::IOLoop->is_running; ``` Set `$server` to the URL of your renderer including the `$baseURL`. With the develop branch the above script will output "success" followed by the contents of the `/etc/passwd` file on your server. With this branch it will output "error" followed by "Not found".
1 parent d5a1000 commit 7c3f9a2

File tree

1 file changed

+8
-7
lines changed

1 file changed

+8
-7
lines changed

lib/RenderApp/Controller/StaticFiles.pm

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,33 +3,34 @@ use Mojo::Base 'Mojolicious::Controller', -signatures;
33

44
use Mojo::File qw(path);
55

6-
sub reply_with_file_if_readable ($c, $file) {
7-
if (-r $file) {
8-
return $c->reply->file($file);
6+
sub reply_with_file_if_readable ($c, $directory, $file) {
7+
my $filePath = $directory->child($file);
8+
if (-r $filePath && $filePath->realpath =~ /^$directory/) {
9+
return $c->reply->file($filePath);
910
} else {
1011
return $c->render(data => 'File not found', status => 404);
1112
}
1213
}
1314

1415
# Route requests for pg_files/CAPA_Graphics to render root Contrib/CAPA/CAPA_Graphics
1516
sub CAPA_graphics_file ($c) {
16-
return $c->reply_with_file_if_readable($c->app->home->child('Contrib/CAPA/CAPA_Graphics', $c->stash('static')));
17+
return $c->reply_with_file_if_readable($c->app->home->child('Contrib/CAPA/CAPA_Graphics'), $c->stash('static'));
1718
}
1819

1920
# Route requests for pg_files to the render root tmp. The
2021
# only requests should be for files in the temporary directory.
2122
# FIXME: Perhaps this directory should be configurable.
2223
sub temp_file ($c) {
23-
$c->reply_with_file_if_readable($c->app->home->child('tmp', $c->stash('static')));
24+
return $c->reply_with_file_if_readable($c->app->home->child('tmp'), $c->stash('static'));
2425
}
2526

2627
# Route request to pg_files to lib/PG/htdocs.
2728
sub pg_file ($c) {
28-
$c->reply_with_file_if_readable(path($ENV{PG_ROOT}, 'htdocs', $c->stash('static')));
29+
return $c->reply_with_file_if_readable(path($ENV{PG_ROOT}, 'htdocs'), $c->stash('static'));
2930
}
3031

3132
sub public_file ($c) {
32-
$c->reply_with_file_if_readable($c->app->home->child('public', $c->stash('static')));
33+
return $c->reply_with_file_if_readable($c->app->home->child('public'), $c->stash('static'));
3334
}
3435

3536
1;

0 commit comments

Comments
 (0)