From 437ab5f76565403b3403438d2e555d11f8128d32 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 13 Sep 2026 03:42:20 +0000 Subject: [PATCH] web: Do not follow symlinks out of the static directory StaticFileHandler.validate_absolute_path checked the requested path against the configured root using os.path.abspath, which does not resolve symlinks. A symlink inside the static directory could therefore point anywhere on the filesystem and its contents would be served: both a symlinked file and a symlinked directory were confirmed to expose files outside the root. Resolve the path with os.path.realpath and require the result to remain inside a new allowed_symlink_directory, which defaults to the served path. Deployments that use such symlinks deliberately can widen it to a directory containing all of the intended targets. Both this check and the existing one on the path as written raise the same error, so a caller cannot tell which way a path left the served directory. The two checks are not merged, and the resolution stays after the check on the unresolved path. That check must operate on the path as written: when allowed_symlink_directory is wider than the root, comparing the resolved path instead would let a ../ traversal out of the root while rejecting the deliberate symlinks the setting exists to permit. Doing the cheap string comparison first also avoids walking the filesystem for a path that is already out of bounds. Because default_filename is appended after the path has been resolved, the joined path is resolved again: the directory containing an index file may be legitimate while the index file itself is a symlink pointing elsewhere. Deferring to a single resolution after that block would be cheaper, but the isdir() check and the redirect it can trigger would then run on a path that had already escaped, and the difference between a 301 and a 403 would reveal whether an out-of-bounds directory exists. A test pins this. The second resolution only happens for directory-index requests; a request for a file resolves once. realpath() is applied to the allowed directory as well as to the requested path, so that serving a directory that is itself reached through a symlink continues to work instead of rejecting every request. strict=True is not used: it is unavailable on Python 3.9, and a path that does not exist resolves to itself and is then reported as a 404. This check reflects the filesystem at one instant, and a path swapped for a symlink afterwards would not be caught; closing that race fully would mean holding descriptors across the whole operation. Short of that, the validated path is now stat'ed once and the result reused for the Content-Length and Last-Modified headers, replacing the separate os.path.exists and os.path.isfile calls. That cuts the number of times a single request resolves the path from ten to six, and ensures the headers describe the file that was validated. The residual race is documented rather than papered over. CVE: CVE-2026-103263 Upstream-Status: Backport [https://github.com/tornadoweb/tornado/commit/437ab5f76565403b3403438d2e555d11f8128d32] Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01JD7LUuFMdyPJB1Dwdq6fPg Signed-off-by: Yogita Urade --- tornado/test/web_test.py | 170 +++++++++++++++++++++++++++++++++++++++ tornado/web.py | 100 ++++++++++++++++++++++- 2 files changed, 267 insertions(+), 3 deletions(-) diff --git a/tornado/test/web_test.py b/tornado/test/web_test.py index 27e7fcfe..8a014d22 100644 --- a/tornado/test/web_test.py +++ b/tornado/test/web_test.py @@ -54,7 +54,9 @@ import itertools import logging import os import re +import shutil import socket +import tempfile import typing # noqa: F401 import unittest import urllib.parse @@ -1510,6 +1512,14 @@ class StaticFileTest(WebTestCase): response = self.get_and_head("/static/blarg") self.assertEqual(response.code, 404) + def test_static_directory_without_default_filename(self): + # Without a default_filename, a directory is not servable. This + # shares the stat() that validate_absolute_path performs, so it is + # worth pinning the status it produces. + with ExpectLog(gen_log, ".*is not a file"): + response = self.get_and_head("/static/dir/") + self.assertEqual(response.code, 403) + def test_path_traversal_protection(self): # curl_httpclient processes ".." on the client side, so we # must test this with simple_httpclient. @@ -1536,6 +1546,166 @@ class StaticFileTest(WebTestCase): self.assertEqual(response.code, 200) +@unittest.skipIf(os.name != "posix", "non-posix OS") +class StaticFileSymlinkTest(WebTestCase): + """Symlinks must not be followed out of the static directory unless the + application opts in with ``allowed_symlink_directory``. + """ + + def setUp(self): + self.tmpdir = tempfile.mkdtemp() + self.addCleanup(shutil.rmtree, self.tmpdir) + self.root = os.path.join(self.tmpdir, "static") + self.outside = os.path.join(self.tmpdir, "outside") + os.mkdir(self.root) + os.mkdir(self.outside) + with open(os.path.join(self.root, "inside.txt"), "w") as f: + f.write("inside") + with open(os.path.join(self.outside, "secret.txt"), "w") as f: + f.write("secret") + with open(os.path.join(self.root, "index.html"), "w") as f: + f.write("root index") + # A symlink to a file outside the root. + os.symlink( + os.path.join(self.outside, "secret.txt"), + os.path.join(self.root, "link.txt"), + ) + # A symlink to a directory outside the root. + os.symlink(self.outside, os.path.join(self.root, "linkdir")) + # Out-of-bounds symlink targets, one existing and one not, for the + # existence-oracle test below. + os.mkdir(os.path.join(self.outside, "realdir")) + os.symlink( + os.path.join(self.outside, "realdir"), + os.path.join(self.root, "link_exists"), + ) + os.symlink( + os.path.join(self.outside, "nope"), + os.path.join(self.root, "link_missing"), + ) + # A symlink that stays inside the root. + os.symlink( + os.path.join(self.root, "inside.txt"), + os.path.join(self.root, "internal.txt"), + ) + super().setUp() + + def get_handlers(self): + return [ + ("/default/(.*)", StaticFileHandler, dict(path=self.root)), + ( + "/permissive/(.*)", + StaticFileHandler, + dict(path=self.root, allowed_symlink_directory=self.tmpdir), + ), + ( + "/default_filename/(.*)", + StaticFileHandler, + dict(path=self.root, default_filename="index.html"), + ), + ] + + def get_app_kwargs(self): + return dict() + + def test_regular_file(self): + response = self.fetch("/default/inside.txt") + self.assertEqual(response.code, 200) + self.assertEqual(response.body, b"inside") + + def test_symlink_within_root(self): + # A symlink is fine as long as it resolves inside the root. + response = self.fetch("/default/internal.txt") + self.assertEqual(response.code, 200) + self.assertEqual(response.body, b"inside") + + def test_symlinked_file_escaping_root(self): + with ExpectLog(gen_log, ".*is not in root static directory"): + response = self.fetch("/default/link.txt") + self.assertEqual(response.code, 403) + + def test_symlinked_directory_escaping_root(self): + with ExpectLog(gen_log, ".*is not in root static directory"): + response = self.fetch("/default/linkdir/secret.txt") + self.assertEqual(response.code, 403) + + def test_served_directory_itself(self): + # A request for the served directory itself resolves to the allowed + # directory exactly, rather than to something beneath it. That must + # still be allowed. + response = self.fetch("/default_filename/") + self.assertEqual(response.code, 200) + self.assertEqual(response.body, b"root index") + + def test_symlinked_default_filename(self): + # The default filename is appended after the directory has been + # resolved, so it must be checked in its own right. + os.mkdir(os.path.join(self.root, "dir")) + os.symlink( + os.path.join(self.outside, "secret.txt"), + os.path.join(self.root, "dir", "index.html"), + ) + with ExpectLog(gen_log, ".*is not in root static directory"): + response = self.fetch("/default_filename/dir/") + self.assertEqual(response.code, 403) + + def test_no_existence_oracle_for_symlinked_directory(self): + # A symlink pointing out of bounds must be rejected identically + # whether or not its target exists, so that it cannot be used to + # probe the server's filesystem. This pins the ordering in + # validate_absolute_path: resolving only after the isdir() check + # would let an existing directory trigger a redirect (301) while a + # missing one still gave 403. + codes = [] + for name in ("link_exists", "link_missing"): + with ExpectLog(gen_log, ".*is not in root static directory"): + codes.append( + self.fetch("/default_filename/" + name, follow_redirects=False).code + ) + self.assertEqual(codes, [403, 403]) + + def test_allowed_symlink_directory(self): + # With an explicit allowed_symlink_directory that contains the + # target, the same symlinks are served. + response = self.fetch("/permissive/link.txt") + self.assertEqual(response.code, 200) + self.assertEqual(response.body, b"secret") + + response = self.fetch("/permissive/linkdir/secret.txt") + self.assertEqual(response.code, 200) + self.assertEqual(response.body, b"secret") + + +class StaticFileSymlinkedRootTest(WebTestCase): + """The root itself may be reached through a symlink; that must not make + every request look like an escape. + """ + + def setUp(self): + self.tmpdir = tempfile.mkdtemp() + self.addCleanup(shutil.rmtree, self.tmpdir) + real_root = os.path.join(self.tmpdir, "real") + os.mkdir(real_root) + with open(os.path.join(real_root, "inside.txt"), "w") as f: + f.write("inside") + # Serve through a symlink to the real directory. + self.root = os.path.join(self.tmpdir, "link") + os.symlink(real_root, self.root) + super().setUp() + + def get_handlers(self): + return [("/static/(.*)", StaticFileHandler, dict(path=self.root))] + + def get_app_kwargs(self): + return dict() + + @unittest.skipIf(os.name != "posix", "non-posix OS") + def test_symlinked_root(self): + response = self.fetch("/static/inside.txt") + self.assertEqual(response.code, 200) + self.assertEqual(response.body, b"inside") + + class StaticDefaultFilenameTest(WebTestCase): def get_app_kwargs(self): return dict( diff --git a/tornado/web.py b/tornado/web.py index 6f1ae304..2c8a1e1d 100644 --- a/tornado/web.py +++ b/tornado/web.py @@ -72,6 +72,7 @@ import numbers import os.path import re import socket +import stat import sys import threading import time @@ -2736,6 +2737,13 @@ class StaticFileHandler(RequestHandler): in your application settings, or add ``default_filename`` as an initializer argument for your ``StaticFileHandler``. + Symlinks are not followed out of the directory being served: if a path + resolves to a location outside it, the request is rejected with a 403 + error. Deployments that deliberately serve content through symlinks + pointing elsewhere can set the ``allowed_symlink_directory`` initializer + argument to a directory containing all of the intended targets. It + defaults to the ``path`` argument. + To maximize the effectiveness of browser caching, this class supports versioned urls (by default using the argument ``?v=``). If a version is given, we instruct the browser to cache this file indefinitely. @@ -2774,6 +2782,11 @@ class StaticFileHandler(RequestHandler): .. versionchanged:: 3.1 Many of the methods for subclasses were added in Tornado 3.1. + + .. versionchanged:: 6.5.9 + Symlinks pointing outside the served directory are no longer + followed, and the ``allowed_symlink_directory`` argument was added + to configure this. """ CACHE_MAX_AGE = 86400 * 365 * 10 # 10 years @@ -2781,9 +2794,17 @@ class StaticFileHandler(RequestHandler): _static_hashes = {} # type: Dict[str, Optional[str]] _lock = threading.Lock() # protects _static_hashes - def initialize(self, path: str, default_filename: Optional[str] = None) -> None: + def initialize( + self, + path: str, + default_filename: Optional[str] = None, + allowed_symlink_directory: Optional[str] = None, + ) -> None: self.root = path self.default_filename = default_filename + if allowed_symlink_directory is None: + allowed_symlink_directory = path + self.allowed_symlink_directory = allowed_symlink_directory @classmethod def reset(cls) -> None: @@ -2995,6 +3016,15 @@ class StaticFileHandler(RequestHandler): # 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) + # The check above is on the path as written, which is cheap and + # rejects traversal without touching the filesystem. Resolve + # symlinks only once it has passed, so that the remaining checks - + # and the path this method returns - refer to the file that will + # actually be opened rather than to the link. The two checks + # cannot be merged: this one must stay on the unresolved path, + # because allowed_symlink_directory may be wider than the root and + # would then let a ../ traversal through. + absolute_path = self._resolve_symlink_target(absolute_path) if os.path.isdir(absolute_path) and self.default_filename is not None: # need to look at the request.path here for when path is empty # but there is some prefix to the path that was already @@ -3012,12 +3042,72 @@ class StaticFileHandler(RequestHandler): self.redirect(self.request.path + "/", permanent=True) return None absolute_path = os.path.join(absolute_path, self.default_filename) - if not os.path.exists(absolute_path): + # The default filename may be a symlink even when the directory + # containing it is not. Resolving here rather than deferring a + # single resolution until after this block is deliberate: the + # isdir() above, and the redirect it can trigger, must not run + # on a path that has already escaped, or the difference between + # a redirect and a 403 would reveal whether an out-of-bounds + # directory exists. + absolute_path = self._resolve_symlink_target(absolute_path) + # Stat the resolved path once and keep the result, instead of + # letting os.path.exists, os.path.isfile and the later header + # generation each resolve the path again. Every one of those is a + # separate trip through the filesystem that could observe a + # different file from the one validated here. + try: + stat_result = os.stat(absolute_path) + except OSError: + # Matches the previous os.path.exists() check, which is also + # false when the path cannot be stat'ed at all. raise HTTPError(404) - if not os.path.isfile(absolute_path): + if not stat.S_ISREG(stat_result.st_mode): raise HTTPError(403, "%s is not a file", self.path) + self._stat_result = stat_result return absolute_path + def _resolve_symlink_target(self, absolute_path: str) -> str: + """Resolves symlinks in ``absolute_path`` and validates the result. + + The path checks in `validate_absolute_path` operate on the path as + written, which says nothing about where a symlink points: a symlink + inside the static directory can name any file on the system. The + resolved path must therefore stay inside + ``allowed_symlink_directory``, which defaults to the directory being + served. + + Raises `HTTPError` (403) if the resolved path escapes that directory. + + Note that this is a check against the state of the filesystem at + one moment: a path that is replaced by a symlink after this returns + but before the file is opened would not be caught. Closing that + race entirely would require holding file descriptors across the + whole operation. This check is not a substitute for filesystem + permissions on a directory that untrusted users can write to. + """ + # realpath() is applied to the allowed directory as well, because it + # may itself be reached through a symlink (a static directory of + # /var/www that links to /srv/www, say). Comparing a resolved path + # against an unresolved directory would reject every request. + allowed_directory = os.path.realpath(self.allowed_symlink_directory) + if not allowed_directory.endswith(os.path.sep): + # As in validate_absolute_path, the separator must not be + # doubled when the directory is the filesystem root. + allowed_directory += os.path.sep + # strict=True is deliberately not used here: it is unavailable on + # Python 3.9, and a path that does not exist resolves to itself, + # which the caller then reports as a 404. + resolved_path = os.path.realpath(absolute_path) + # The trailing separator lets the allowed directory itself match, as + # well as anything beneath it. This is reachable because the path may + # still be a directory at this point (when default_filename is set). + if not (resolved_path + os.path.sep).startswith(allowed_directory): + # Deliberately the same error as the check on the path as + # written, so that a caller cannot tell which way a path left + # the served directory. + raise HTTPError(403, "%s is not in root static directory", self.path) + return resolved_path + @classmethod def get_content( cls, abspath: str, start: Optional[int] = None, end: Optional[int] = None @@ -3077,6 +3167,10 @@ class StaticFileHandler(RequestHandler): def _stat(self) -> os.stat_result: assert self.absolute_path is not None + # validate_absolute_path normally populates this, so that the size + # and modification time reported here describe the same file that + # was validated. Subclasses that override validate_absolute_path + # without calling super() fall back to stat'ing here. if not hasattr(self, "_stat_result"): self._stat_result = os.stat(self.absolute_path) return self._stat_result -- 2.35.6