Skip to content

Commit b6b1a24

Browse files
xsawyerxSysPete
authored andcommitted
Resolve relative paths in send_file:
If someone were to send a file to send_file() that includes '../', then we would allow them to reach outside the directory we choose. This is a possible security issue. (One can argue the user should sanitize their input, but I think we simply shouldn't allow it.) The problem is that Path::Tiny does the right thing and allows us to reach there. To prevent that, we're resolving paths using Path::Tiny's realpath() method and then subsumes() to see that the file is within the original directory. Otherwise, we send a 403 forbidden. There is also a test that verifies this is done correctly.
1 parent 00b1bda commit b6b1a24

File tree

2 files changed

+22
-1
lines changed

2 files changed

+22
-1
lines changed

lib/Dancer2/Core/App.pm

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1043,8 +1043,14 @@ sub send_file {
10431043
$self->with_return->( $self->response );
10441044
};
10451045

1046-
$file_path = Path::Tiny::path( $dir, $path );
1046+
# resolve relative paths (with '../') as much as possible
1047+
$file_path = Path::Tiny::path( $dir, $path )->realpath;
10471048

1049+
# We need to check whether they are trying to access
1050+
# a directory outside their scope
1051+
$err_response->(403) if !Path::Tiny::path($dir)->realpath->subsumes($file_path);
1052+
1053+
# other error checks
10481054
$err_response->(403) if !$file_path->exists;
10491055
$err_response->(404) if !$file_path->is_file;
10501056
$err_response->(403) if !-r $file_path;

t/dsl/send_file.t

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,10 @@ use Ref::Util qw<is_coderef>;
2323
send_file 'index.html';
2424
};
2525

26+
get '/illegal' => sub {
27+
send_file '../index.html';
28+
};
29+
2630
prefix '/some' => sub {
2731
get '/image' => sub {
2832
send_file '1x1.png';
@@ -144,6 +148,17 @@ test_psgi $app, sub {
144148
ok($r->is_success, 'send_file returns success');
145149
is($r->header('Content-Disposition'), 'inline; filename="1x1.png"', 'send_file returns correct inline Content-Disposition');
146150
};
151+
152+
subtest "Illegal path" => sub {
153+
my $r = $cb->( GET '/illegal' );
154+
is( $r->code, 403, 'Illegal path returns 403' );
155+
is(
156+
$r->content,
157+
'Forbidden',
158+
'Text content contains UTF-8 characters',
159+
);
160+
};
161+
147162
};
148163

149164
done_testing;

0 commit comments

Comments
 (0)