mirror of
https://github.com/got-feedBack/feedBack.git
synced 2026-07-21 20:31:21 +00:00
e729c44d5b
`Path.resolve()` is a filesystem call — it lstats every component of the path.
`_resolve_dlc_path` and `safe_join` both re-resolved their ROOT on every single
call, and those run once per song, per art fetch, per scanned row.
Found while profiling a 2-fps report: on a real 50,944-song library the server
was issuing ~23,500 stat/lstat calls per second, re-walking the same three
parent directories over and over, and burning ~50% of a core doing it. It is
worst exactly where big libraries live — the library was on an NTFS-3G (FUSE)
mount, where every stat is a userspace round trip through mount.ntfs-3g (itself
visible in top). The cost was the constant re-resolution, not the work.
A root is fixed for the life of the process, so resolve it once
(safepath.resolved_root, lru_cache). Measured, 5,000 lookups against a real
library path:
before: 15,264 stat syscalls (54.2 ms)
after: 277 stat syscalls ( 0.8 ms) 55x fewer
Containment is unchanged, which is the part that matters:
- safe_join still resolves the CANDIDATE on every call — following its
symlinks IS the zip-slip / traversal defence, so it is never cached. Only
the server-owned root is.
- _resolve_dlc_path keeps its lexical containment check (deliberately does not
follow symlinks, so junction-mounted libraries keep working).
Tradeoff, documented on resolved_root: if a root's symlink is re-pointed at a
NEW target while the server runs, the old target stays in effect until restart.
Fine for a library path fixed at startup; the cache is keyed on the Path, so
switching library dir is a different key.
Tests: root resolved once across 500 lookups (the regression), a different root
is a different entry, and the containment contract re-pinned — traversal,
Windows drive-absolute, backslash, NUL, empty, and a symlink escaping the root
must still be refused. Full suite green.
73 lines
3.1 KiB
Python
73 lines
3.1 KiB
Python
"""Path-containment helper for code that joins attacker-controlled names
|
|
under a server-owned root.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
from functools import lru_cache
|
|
from pathlib import Path
|
|
|
|
|
|
@lru_cache(maxsize=16)
|
|
def resolved_root(root: Path) -> Path:
|
|
"""Canonical (link-resolved) form of a server-owned root directory.
|
|
|
|
``Path.resolve()`` is a filesystem call: it lstats every component of the
|
|
path. The roots we join against — the DLC library, a plugin's asset dir —
|
|
are fixed for the life of the process, but the containment helpers below
|
|
(and ``dlc_paths._resolve_dlc_path``) were re-resolving them on EVERY call,
|
|
and those are called once per song, per art fetch, per scanned row.
|
|
|
|
On a real 50,944-song library that cost ~23,500 stat/lstat calls per second,
|
|
pinning a core. It is brutal when the library lives on a FUSE mount
|
|
(NTFS-3G, SMB, sshfs), where every stat is a userspace round trip: the same
|
|
three parent directories were being walked over and over.
|
|
|
|
Cached because a root is a constant here, not because resolution is cheap.
|
|
Consequence: if a root's symlink/junction is re-pointed at a NEW target
|
|
while the server is running, the old target stays in effect until restart.
|
|
That is fine for a library path fixed at startup, and the cache is keyed on
|
|
the Path, so switching to a different library dir is a different key.
|
|
"""
|
|
return root.resolve()
|
|
|
|
|
|
def safe_join(root: Path, name: str) -> Path | None:
|
|
"""Resolve ``name`` under ``root`` and return the resolved Path, or
|
|
``None`` if it would escape ``root`` or is unrepresentable.
|
|
|
|
Rejects:
|
|
* empty names
|
|
* paths that resolve outside ``root`` (``..`` traversal, absolute paths)
|
|
* paths the OS can't resolve (embedded NULs, OSError on stat)
|
|
|
|
Normalizes:
|
|
* backslash separators to forward slash so a Windows-style entry
|
|
name inside a user-supplied archive can't bypass containment on
|
|
POSIX hosts (``..\\foo`` would otherwise be treated as a literal
|
|
single filename on Linux and resolve inside ``root`` — but on
|
|
Windows the same string IS a traversal; normalising means both
|
|
platforms reject it identically).
|
|
"""
|
|
if not name:
|
|
return None
|
|
# Reject embedded NULs explicitly. This used to ride on `.resolve()`
|
|
# raising ValueError, but on Python 3.13 (Windows) resolve() no longer
|
|
# raises for an embedded NUL, so the byte would otherwise leak through
|
|
# containment. An explicit guard is strictly-more-rejection (no effect on
|
|
# the zip-slip / traversal contract).
|
|
if "\x00" in name:
|
|
return None
|
|
safe = name.replace("\\", "/")
|
|
try:
|
|
# The ROOT is a constant — resolve it once (see resolved_root). The
|
|
# CANDIDATE must still be resolved on every call: following its symlinks
|
|
# is exactly the zip-slip / traversal defence, so it is never cached.
|
|
root_res = resolved_root(root)
|
|
candidate = (root_res / safe).resolve()
|
|
if not candidate.is_relative_to(root_res):
|
|
return None
|
|
except (ValueError, OSError):
|
|
return None
|
|
return candidate
|