From ecb3ea7543cc942659faf3d2144853018afa6139 Mon Sep 17 00:00:00 2001 From: Ben Darnell Date: Fri, 17 Jul 2015 11:36:53 -0400 Subject: [PATCH] Fix path traversal check in StaticFileHandler. 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 7b03cd62fb and 60952528. --- MANIFEST.in | 1 + setup.py | 1 + tornado/test/static_foo.txt | 2 ++ tornado/test/web_test.py | 9 +++++++++ tornado/web.py | 10 +++++++--- 5 files changed, 20 insertions(+), 3 deletions(-) create mode 100644 tornado/test/static_foo.txt diff --git a/MANIFEST.in b/MANIFEST.in index 41050ad61..353f222fa 100644 --- a/MANIFEST.in +++ b/MANIFEST.in @@ -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 diff --git a/setup.py b/setup.py index 9b81b2b8b..9e5ea7fa2 100644 --- a/setup.py +++ b/setup.py @@ -147,6 +147,7 @@ setup( "options_test.cfg", "static/robots.txt", "static/dir/index.html", + "static_foo.txt", "templates/utf8.html", "test.crt", "test.key", diff --git a/tornado/test/static_foo.txt b/tornado/test/static_foo.txt new file mode 100644 index 000000000..bdb44f391 --- /dev/null +++ b/tornado/test/static_foo.txt @@ -0,0 +1,2 @@ +This file should not be served by StaticFileHandler even though +its name starts with "static". diff --git a/tornado/test/web_test.py b/tornado/test/web_test.py index 96edd6c24..9374c4824 100644 --- a/tornado/test/web_test.py +++ b/tornado/test/web_test.py @@ -1181,6 +1181,15 @@ class StaticFileTest(WebTestCase): 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): diff --git a/tornado/web.py b/tornado/web.py index 0a50f7935..9847bb02e 100644 --- a/tornado/web.py +++ b/tornado/web.py @@ -2376,9 +2376,13 @@ class StaticFileHandler(RequestHandler): .. 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) -- 2.47.2