Skip to content

Commit

Permalink
Fix path traversal check in StaticFileHandler.
Browse files Browse the repository at this point in the history
Previously StaticFileHandler would allow access to files whose name
starts with the static root directory, not just those that are actually
in the directory.

The bug was introduced in Tornado 3.1 via commits 7b03cd6 and
6095252.
  • Loading branch information
bdarnell committed Jul 17, 2015
1 parent fdfaf3d commit ecb3ea7
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 3 deletions.
1 change: 1 addition & 0 deletions MANIFEST.in
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ include tornado/test/gettext_translations/fr_FR/LC_MESSAGES/tornado_test.po
include tornado/test/options_test.cfg
include tornado/test/static/robots.txt
include tornado/test/static/dir/index.html
include tornado/test/static_foo.txt
include tornado/test/templates/utf8.html
include tornado/test/test.crt
include tornado/test/test.key
Expand Down
1 change: 1 addition & 0 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ def build_extension(self, ext):
"options_test.cfg",
"static/robots.txt",
"static/dir/index.html",
"static_foo.txt",
"templates/utf8.html",
"test.crt",
"test.key",
Expand Down
2 changes: 2 additions & 0 deletions tornado/test/static_foo.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
This file should not be served by StaticFileHandler even though
its name starts with "static".
9 changes: 9 additions & 0 deletions tornado/test/web_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -1181,6 +1181,15 @@ def test_static_404(self):
response = self.get_and_head('/static/blarg')
self.assertEqual(response.code, 404)

def test_path_traversal_protection(self):
with ExpectLog(gen_log, ".*not in root static directory"):
response = self.get_and_head('/static/../static_foo.txt')
# Attempted path traversal should result in 403, not 200
# (which means the check failed and the file was served)
# or 404 (which means that the file didn't exist and
# is probably a packaging error).
self.assertEqual(response.code, 403)


@wsgi_safe
class StaticDefaultFilenameTest(WebTestCase):
Expand Down
10 changes: 7 additions & 3 deletions tornado/web.py
Original file line number Diff line number Diff line change
Expand Up @@ -2376,9 +2376,13 @@ def validate_absolute_path(self, root, absolute_path):
.. versionadded:: 3.1
"""
root = os.path.abspath(root)
# os.path.abspath strips a trailing /
# it needs to be temporarily added back for requests to root/
# os.path.abspath strips a trailing /.
# We must add it back to `root` so that we only match files
# in a directory named `root` instead of files starting with
# that prefix.
root = os.path.abspath(root) + os.path.sep
# The trailing slash also needs to be temporarily added back
# the requested path so a request to root/ will match.
if not (absolute_path + os.path.sep).startswith(root):
raise HTTPError(403, "%s is not in root static directory",
self.path)
Expand Down

0 comments on commit ecb3ea7

Please sign in to comment.