Restore: on Windows, member names with backslashes or a drive letter are written outside the media directory #478

New issue

Closed

opened 2026-10-01 10:46:59 +00:00 by tiagoagueda · 0 comments

tiagoagueda commented

2026-10-01 10:46:59 +00:00

Owner

Found on 2026-09-29 in a read-only review of postulo at 95348908d, running _safe_member_path from the snapshot on Windows with hand-written member names.

Checked again on 2026-10-01 against main at 0a5cad587: still present.

_safe_member_path checks an archive member's name as a POSIX path, refusing .. and empty parts split on /. restore_backup then turns the result into a native path with Path(*relative.parts). On Windows a single POSIX part may contain backslashes and a drive, which the native path treats as separators and an anchor, so the destination leaves MEDIA_ROOT or POSTULO_PLUGINS_DIR. The code promises that "a hostile archive writes nothing at all" (core/backup.py:526-527), and Postulo is run on Windows for development (the README describes its Windows fallback renderer).

Where: core/backup.py:460 _safe_member_path(); core/backup.py:546 destination = roots[prefix] / Path(*relative.parts) in restore_backup().
What happens: run on the snapshot with MEDIA_ROOT at C:\data\media:
- media/..\..\evil.txt passes the check and resolves to C:\evil.txt;
- media/C:\Windows\evil.txt passes and becomes C:\Windows\evil.txt, because a drive-qualified part replaces the root;
- media/a/..\..\..\x passes and resolves to C:\x.
  Only media/../x is refused, which is the one shape tests/test_backup.py:263 tests.
Who can trigger it: somebody who gets an operator on Windows to restore an archive they built or altered. The operator must run restore themselves; a restored archive can already install plugin code that loads at the next start, so this widens what a hostile archive reaches rather than creating a new way in.
Why it matters: the path check is the one boundary between an archive's contents and the rest of the file system, and it holds only on POSIX.
Fix: check the destination rather than the name. Build destination, then refuse unless destination.resolve() is under roots[prefix].resolve() (Path.is_relative_to), before anything is written; and also refuse any part containing \ or : so a name means the same on every platform.

Done when

tests/test_backup.py: test_a_hostile_archive_writes_nothing is parametrised with media/..\..\escaped.txt, media/C:\escaped.txt and plugins/a/..\..\..\escaped.txt, and each is refused before the database is loaded, on every platform.

Found on 2026-09-29 in a read-only review of postulo at 95348908d, running `_safe_member_path` from the snapshot on Windows with hand-written member names. Checked again on 2026-10-01 against `main` at `0a5cad587`: still present. `_safe_member_path` checks an archive member's name as a POSIX path, refusing `..` and empty parts split on `/`. `restore_backup` then turns the result into a native path with `Path(*relative.parts)`. On Windows a single POSIX part may contain backslashes and a drive, which the native path treats as separators and an anchor, so the destination leaves `MEDIA_ROOT` or `POSTULO_PLUGINS_DIR`. The code promises that "a hostile archive writes nothing at all" (`core/backup.py:526-527`), and Postulo is run on Windows for development (the README describes its Windows fallback renderer). - **Where:** `core/backup.py:460` `_safe_member_path()`; `core/backup.py:546` `destination = roots[prefix] / Path(*relative.parts)` in `restore_backup()`. - **What happens:** run on the snapshot with `MEDIA_ROOT` at `C:\data\media`: - `media/..\..\evil.txt` passes the check and resolves to `C:\evil.txt`; - `media/C:\Windows\evil.txt` passes and becomes `C:\Windows\evil.txt`, because a drive-qualified part replaces the root; - `media/a/..\..\..\x` passes and resolves to `C:\x`. Only `media/../x` is refused, which is the one shape `tests/test_backup.py:263` tests. - **Who can trigger it:** somebody who gets an operator on Windows to restore an archive they built or altered. The operator must run `restore` themselves; a restored archive can already install plugin code that loads at the next start, so this widens what a hostile archive reaches rather than creating a new way in. - **Why it matters:** the path check is the one boundary between an archive's contents and the rest of the file system, and it holds only on POSIX. - **Fix:** check the destination rather than the name. Build `destination`, then refuse unless `destination.resolve()` is under `roots[prefix].resolve()` (`Path.is_relative_to`), before anything is written; and also refuse any part containing `\` or `:` so a name means the same on every platform. ## Done when - `tests/test_backup.py`: `test_a_hostile_archive_writes_nothing` is parametrised with `media/..\..\escaped.txt`, `media/C:\escaped.txt` and `plugins/a/..\..\..\escaped.txt`, and each is refused before the database is loaded, on every platform.

tiagoagueda added this to the 0.5.0 milestone

2026-10-01 10:46:59 +00:00

tiagoagueda added the

security

enhancement

tier

labels

2026-10-01 10:46:59 +00:00

tiagoagueda referenced this issue from a commit

2026-10-03 16:50:56 +00:00

Refuse archive member names a Windows path would read differently

tiagoagueda closed this issue