Repository navigation
web: Harden static_url and get_version - #3784
Merged
Merged
Conversation
static_url() hashed whatever file get_absolute_path pointed at without the checks validate_absolute_path applies when serving. A path with ../ or a symlink pointing outside the static directory would be read and its SHA-512 included in the URL, and a path resolving to a device such as /dev/zero (or a FIFO) would block the event loop indefinitely. get_version now checks that the path is inside static_path, resolves symlinks against allowed_symlink_directory (taken from static_handler_args, defaulting to the static root), and requires a regular file before reading it. The root and symlink-directory logic is factored out of validate_absolute_path so both share it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019nvEV873ZwBncgxPP13Xfd
Rework the previous commit so that get_version is unchanged and keeps delegating all filesystem knowledge to the methods documented for replacing filesystem access: - get_absolute_path now rejects paths that lead outside the root as written (e.g. ../), sharing the check with validate_absolute_path. - get_content refuses anything that is not a regular file (devices such as /dev/zero or Windows' CON/NUL, FIFOs, directories), sharing the stat check with validate_absolute_path. - get_content_version stops hashing after MAX_VERSION_CONTENT_SIZE (64 MiB by default) so a huge file cannot block the event loop. Symlinks are still only checked by validate_absolute_path, since allowed_symlink_directory is per-handler configuration that the class methods used by static_url cannot see. Tests that don't need symlinks or FIFOs now run on all platforms, and the Windows special filenames are checked for static_url as well as for serving. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019nvEV873ZwBncgxPP13Xfd
make_static_url runs without a handler instance and cannot see allowed_symlink_directory, so a symlink inside the static directory can still cause static_url to hash (but not serve) a file outside it. Fixing that would need a new overridable hook plus heuristics to avoid breaking subclasses that map paths themselves (jupyter_server, voila), so document the limitation instead. Also note in static_url the behavior changes from the previous commits. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019nvEV873ZwBncgxPP13Xfd
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
static_urlandget_versionnow check for traversal outside the static root directory via things like.., and prohibit access to anything but regular files (devices, fifos, etc).get_versionhas a cap on the size of file it will hash to avoid blocking the IOLoop for too long (defaulting to 64MB. Large files can still be accessed, they just won't have thev=argument and associated caching changes).Document the limitation that because the
allowed_symlink_directorysetting isn't accessible on this code path, we do not validate symlink targets in get_version (so an attacker who can control symlinks in the static file directory and can cause them to be passed tostatic_urlcan determine the existence of files and their hashes).