fix Windows path traversal bug (closes #738)

mojolicious · Feb 2, 2015 · 9ffa38f · 9ffa38f
1 parent 0cc15ac
commit 9ffa38f
Show file tree

Hide file tree

Showing 7 changed files with 49 additions and 3 deletions.
diff --git a/Changes b/Changes
@@ -1,5 +1,7 @@
 
 5.76  2015-02-02
+  - Emergency release for a critical security issue that can expose files on
+    Windows systems, everybody should update!
   - Increased default max_message_size from 10MB to 16MB in Mojo::Message.
   - Reduced default max_line_size from 10KB to 8KB in Mojo::Headers and
     Mojo::Message.

diff --git a/lib/Mojo/Path.pm b/lib/Mojo/Path.pm
@@ -15,7 +15,7 @@ sub canonicalize {
 
   my $parts = $self->parts;
   for (my $i = 0; $i <= $#$parts;) {
-    if ($parts->[$i] eq '.' || $parts->[$i] eq '') { splice @$parts, $i, 1 }
+    if ($parts->[$i] =~ /^(?:\.||\.{3,})$/) { splice @$parts, $i, 1 }
     elsif ($i < 1 || $parts->[$i] ne '..' || $parts->[$i - 1] eq '..') { $i++ }
     else { splice @$parts, --$i, 2 }
   }
@@ -169,14 +169,22 @@ following new ones.
 
   $path = $path->canonicalize;
 
-Canonicalize path.
+Canonicalize path, parts comprised solely of three or more dots are treated as
+a single dot, to protect certain operating systems from path traversal
+attacks.
 
   # "/foo/baz"
   Mojo::Path->new('/foo/./bar/../baz')->canonicalize;
 
   # "/../baz"
   Mojo::Path->new('/foo/../bar/../../baz')->canonicalize;
 
+  # "/foo/bar"
+  Mojo::Path->new('/foo/.../bar')->canonicalize;
+
+  # "/foo/bar"
+  Mojo::Path->new('/foo/...../bar')->canonicalize;
+
 =head2 clone
 
   my $clone = $path->clone;

diff --git a/lib/Mojolicious/Static.pm b/lib/Mojolicious/Static.pm
@@ -32,7 +32,7 @@ sub dispatch {
   $path = $stash->{path} ? $path->new($stash->{path}) : $path->clone;
   return undef unless my @parts = @{$path->canonicalize->parts};
 
-  # Serve static file and prevent directory traversal
+  # Serve static file and prevent path traversal
   return undef if $parts[0] eq '..' || !$self->serve($c, join('/', @parts));
   $stash->{'mojo.static'} = 1;
   return !!$c->rendered;

diff --git a/t/mojo/path.t b/t/mojo/path.t
@@ -88,6 +88,24 @@ is_deeply $path->parts, [qw(.. .. .. .. .. .. .. .. etc passwd)],
 ok $path->leading_slash, 'has leading slash';
 ok !$path->trailing_slash, 'no trailing slash';
 
+# Canonicalize (triple dot)
+$path = Mojo::Path->new('/foo/.../.../windows/win.ini');
+is "$path", '/foo/.../.../windows/win.ini', 'same path';
+is_deeply $path->parts, [qw(foo ... ... windows win.ini)], 'right structure';
+is $path->canonicalize, '/foo/windows/win.ini', 'canonicalized path';
+is_deeply $path->parts, [qw(foo windows win.ini)], 'right structure';
+ok $path->leading_slash, 'has leading slash';
+ok !$path->trailing_slash, 'no trailing slash';
+
+# Canonicalize (more dots)
+$path = Mojo::Path->new('/foo/....../windows/win.ini');
+is "$path", '/foo/....../windows/win.ini', 'same path';
+is_deeply $path->parts, [qw(foo ...... windows win.ini)], 'right structure';
+is $path->canonicalize, '/foo/windows/win.ini', 'canonicalized path';
+is_deeply $path->parts, [qw(foo windows win.ini)], 'right structure';
+ok $path->leading_slash, 'has leading slash';
+ok !$path->trailing_slash, 'no trailing slash';
+
 # Canonicalizing (with escaped "%")
 $path = Mojo::Path->new('%2ftest%2f..%252f..%2f..%2f..%2f..%2fetc%2fpasswd');
 is "$path", '%2ftest%2f..%252f..%2f..%2f..%2f..%2fetc%2fpasswd', 'same path';

diff --git a/t/mojolicious/app.t b/t/mojolicious/app.t
@@ -392,6 +392,12 @@ $t->get_ok('/../../mojolicious/secret.txt')->status_is(404)
   ->header_is(Server => 'Mojolicious (Perl)')
   ->content_like(qr/Page not found/);
 
+# Try to access a file which is not under the web root via path
+# traversal (triple dot)
+$t->get_ok('/.../mojolicious/secret.txt')->status_is(404)
+  ->header_is(Server => 'Mojolicious (Perl)')
+  ->content_like(qr/Page not found/);
+
 # Check If-Modified-Since
 $t->get_ok('/hello.txt' => {'If-Modified-Since' => $mtime})->status_is(304)
   ->header_is(Server => 'Mojolicious (Perl)')->content_is('');

diff --git a/t/mojolicious/production_app.t b/t/mojolicious/production_app.t
@@ -100,6 +100,12 @@ $t->get_ok('/../../mojolicious/secret.txt')->status_is(404)
   ->header_is(Server => 'Mojolicious (Perl)')
   ->content_like(qr/Page not found/);
 
+# Try to access a file which is not under the web root via path
+# traversal in production mode (triple dot)
+$t->get_ok('/.../mojolicious/secret.txt')->status_is(404)
+  ->header_is(Server => 'Mojolicious (Perl)')
+  ->content_like(qr/Page not found/);
+
 # Embedded production static file
 $t->get_ok('/some/static/file.txt')->status_is(200)
   ->header_is(Server => 'Mojolicious (Perl)')

diff --git a/t/mojolicious/testing_app.t b/t/mojolicious/testing_app.t
@@ -45,4 +45,10 @@ $t->get_ok('/../../mojolicious/secret.txt')->status_is(404)
   ->header_is(Server => 'Mojolicious (Perl)')
   ->content_like(qr/Testing not found/);
 
+# Try to access a file which is not under the web root via path
+# traversal in testing mode (triple dot)
+$t->get_ok('/.../mojolicious/secret.txt')->status_is(404)
+  ->header_is(Server => 'Mojolicious (Perl)')
+  ->content_like(qr/Testing not found/);
+
 done_testing();