Extract uploaded theme archives safely
Python · Python · advanced · greenfield
Adds the safe extraction step for uploaded theme archives. Resolves the destination once, then validates every entry's normalized target path against it before a single file is written; absolute paths and `..` traversal both reject with the entry name in the error. Extraction itself is a plain `extractall` after the whole archive has passed.
Themes are user-uploaded ZIPs unpacked on the web host; a traversal entry like `../../app/config.py` would let an upload overwrite server code.
Requirements
- Implement `safe_extract(archive_path, dest_dir)`: extract the uploaded ZIP into `dest_dir`, which is created fresh per upload and contains no pre-existing entries.
- A hostile archive must not be able to write outside `dest_dir` (zip-slip): reject entries with absolute paths, reject any entry containing a `..` path component (even one that would still resolve inside `dest_dir` — legitimate theme archives never contain `..`), and independently verify the resolved target stays under `dest_dir`.
- Rejection is explicit: raise `ValueError` naming the offending entry — do **not** rely on silent name sanitization, because a security scanner must be able to flag the upload, and validate **every** entry before extracting anything (no partial extractions of a hostile archive).
- Archive size and entry-count limits are enforced by the upload endpoint before this function runs; they are out of scope here.
- The service runs on Linux; archives are produced by end users with arbitrary tools, so entry names are untrusted byte strings.
Files touched
- app/themes/extract.py
--- app/themes/extract.py
+from pathlib import Path
+from zipfile import ZipFile
+
+
+def safe_extract(archive_path, dest_dir):
+ """Extract ``archive_path`` under ``dest_dir``, rejecting path traversal."""
+ dest = Path(dest_dir).resolve()
+ with ZipFile(archive_path) as archive:
+ for info in archive.infolist():
+ name = info.filename
+ if Path(name).is_absolute() or ".." in Path(name).parts:
+ raise ValueError(f"unsafe path in archive: {name!r}")
+ target = (dest / name).resolve()
+ if not target.is_relative_to(dest):
+ raise ValueError(f"unsafe path in archive: {name!r}")
+ archive.extractall(dest)
+