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.

diff --git a/MANIFEST.in b/MANIFEST.in
index 41050ad6159297814c6874818f26b2549c351da9..353f222faaa8bd227cfd475ae4a1336cd05803c1 100644 (file)
--- 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 9b81b2b8b5635e9550898340d5722b53d8d74bb1..9e5ea7fa23bd04425515e0e852545d179a5fa758 100644 (file)
--- 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 (file)
index 0000000..bdb44f3
--- /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 96edd6c24226297451feceeca0f6ae1233a3a431..9374c4824b378050fd3b2c7d6c34c4b8e844004d 100644 (file)
--- 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 0a50f79357fb338f4f43088b6abafe79ae2a51cf..9847bb02e93215da78d7d0773b8e8630d57d4631 100644 (file)
--- 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)