From: Ben Darnell <ben@bendarnell.com>
Date: Fri, 17 Jul 2015 15:36:53 +0000 (-0400)
Subject: Fix path traversal check in StaticFileHandler.
X-Git-Tag: v4.2.1~1
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ecb3ea7543cc942659faf3d2144853018afa6139;p=thirdparty%2Ftornado.git

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 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)