PST-044: The owner-folder guard is missing on rendered-document and export downloads, and is a raw prefix test elsewhere #773

New issue

Closed

opened 2026-10-10 10:56:16 +00:00 by tiagoagueda · 1 comment

tiagoagueda commented

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

Owner

Found in the security audit of 2026-10-10 (SECURITY-AUDIT-2026-10-10.md), run against main at c30851b from 8 to 10 October 2026, as PST-044. Severity: Low.

The text below is the audit's entry, unchanged apart from links to the other issues it names. The finding was produced by automated review and checked by a second automated pass; re-check it before acting on it.

Severity: Low
Category: authorization / defence-in-depth (file delivery)
Location: src/postulo/documents/views.py (794-802); src/postulo/api/routers/documents.py (346-350); src/postulo/core/views_export.py (86-94); src/postulo/core/files.py (98-113; 92-95)
Found by: completeness review: chains, completeness review: recent changes, ownership review of api, ownership review of core, ownership review of documents
Related: PST-045 (#774)

Who can do it, and what they need. An authenticated account holder, given one RenderedDocument row of their own whose file column names a path under another account's documents/<other_id>/ folder. No request-driven path writes such a row today (LocalStore.put and the importer always store through upload_to_documents, which prefixes the owner's id), so the precondition is a bug in a future writer, a plugin that saves renders, a careless migration, or a hand-edited database. …

How it works.

A RenderedDocument owned by A has file.name = documents/<B>/2026/10/theirs.pdf (precondition above).
A signs in and requests GET /documents/sent/<pk>/download/.
RenderedDownloadView.get narrows by owner (row is A's) and calls serve_private_file(request, document.file, download_name=...) with no within.
serve_private_file only checks the path stays inside MEDIA_ROOT and streams B's bytes. Verified with a scratch test: the same manipulation on an UploadedDocument row answers 404 (the within=document.storage_prefix check), while the RenderedDocument row answers 200 with the other owner's bytes.

Impact. A second line of defence that exists for uploads (and for API upload downloads) is absent for rendered snapshots, so one wrong row can hand one person another person's file. No live exploit path from a request was found; the finding is that the project's stated #663 guarantee ('a row whose name says otherwise is not handed over whatever it points at') does not hold for half of the file-bearing models.

Evidence.

views.py:800-802
    def get(self, request: HttpRequest, pk: int) -> HttpResponse:
        document = get_object_or_404(self.get_queryset(), pk=pk)
        return serve_private_file(request, document.file, download_name=document.download_name)

views.py:785-790 (uploads, for contrast)
        return serve_private_file(
            request,
            document.file,
            download_name=document.download_name,
            as_attachment=True,
            within=document.storage_prefix,
        )

api/routers/documents.py:346-350
    if source == "rendered":
        document = owned_or_404(request, RenderedDocument.objects, pk)
        return serve_private_file(
            request, docum
…

Verification (verdict: confirmed; severity after verification: Low). Line numbers are right: views.py:794-802 and api/routers/documents.py:346-350. The guarantee quoted from #663 is written in core/files.py about uploads ('an upload is kept under its owner's own'). It is not a promise made for every file-bearing model. The same gap exists for the other serve_private_file callers (jobs/page_views.py CapturedRenderingView, core/views_export.py export archives), so RenderedDocument is not unique. Remediation: pass within=f'documents/{document.owner_id}/' in both rendered branches, or add a storage_prefix property to RenderedDocument, and add a test beside the upload one. No attacker can create the precondition today.

Suggested remediation. Give RenderedDocument the same storage_prefix property (documents/{owner_id}/) and pass within=document.storage_prefix from RenderedDownloadView and from the API's rendered branch, so a snapshot row is held to its owner's folder exactly as an upload is. Add the rendered case to tests/security/test_files.py beside the upload one.

Also covered by this issue.

Rendered-document download does not pin the file to the owner's storage prefix, unlike uploads (defence-in-depth gap) (src/postulo/api/routers/documents.py (346-350); Informational). None on its own; a second mistake elsewhere is required. Recorded because the project treats this check as a rule for uploads and the sent CV/letter is the file it considers most sensitive.
Export archive download does not pass the within owner-folder guard that uploads use (src/postulo/core/views_export.py (86-94); Informational). One wrong file name on one row becomes another person's file. No path reaches it from a request today, so this is a hardening gap rather than an exploitable hole.
Upload ownership guard within= is a raw string-prefix test that .. segments in a stored name defeat (src/postulo/core/files.py (98-113); Informational). If any row ever carries such a name, the #663 promise 'an upload is served only from its owner's folder' does not hold, and a file in another account's folder is handed over. The guard is the second line of defence behind the owner-scoped queryset. …
The 'within' owner-folder guard on private files is a plain startswith and does not normalise '..' segments (src/postulo/core/files.py (92-95); Informational). None today; the guard would not hold in the case it was written for if a traversal name ever reached the column.

Found in the security audit of 2026-10-10 (`SECURITY-AUDIT-2026-10-10.md`), run against `main` at `c30851b` from 8 to 10 October 2026, as **PST-044**. Severity: **Low**. The text below is the audit's entry, unchanged apart from links to the other issues it names. The finding was produced by automated review and checked by a second automated pass; re-check it before acting on it. --- - **Severity:** Low - **Category:** authorization / defence-in-depth (file delivery) - **Location:** `src/postulo/documents/views.py` (794-802); `src/postulo/api/routers/documents.py` (346-350); `src/postulo/core/views_export.py` (86-94); `src/postulo/core/files.py` (98-113; 92-95) - **Found by:** completeness review: chains, completeness review: recent changes, ownership review of api, ownership review of core, ownership review of documents - **Related:** PST-045 (#774) **Who can do it, and what they need.** An authenticated account holder, given one RenderedDocument row of their own whose `file` column names a path under another account's `documents/<other_id>/` folder. No request-driven path writes such a row today (LocalStore.put and the importer always store through `upload_to_documents`, which prefixes the owner's id), so the precondition is a bug in a future writer, a plugin that saves renders, a careless migration, or a hand-edited database. … **How it works.** 1. A RenderedDocument owned by A has `file.name` = `documents/<B>/2026/10/theirs.pdf` (precondition above). 2. A signs in and requests GET /documents/sent/<pk>/download/. 3. `RenderedDownloadView.get` narrows by owner (row is A's) and calls `serve_private_file(request, document.file, download_name=...)` with no `within`. 4. `serve_private_file` only checks the path stays inside MEDIA_ROOT and streams B's bytes. Verified with a scratch test: the same manipulation on an UploadedDocument row answers 404 (the `within=document.storage_prefix` check), while the RenderedDocument row answers 200 with the other owner's bytes. **Impact.** A second line of defence that exists for uploads (and for API upload downloads) is absent for rendered snapshots, so one wrong row can hand one person another person's file. No live exploit path from a request was found; the finding is that the project's stated #663 guarantee ('a row whose name says otherwise is not handed over whatever it points at') does not hold for half of the file-bearing models. **Evidence.** ```text views.py:800-802 def get(self, request: HttpRequest, pk: int) -> HttpResponse: document = get_object_or_404(self.get_queryset(), pk=pk) return serve_private_file(request, document.file, download_name=document.download_name) views.py:785-790 (uploads, for contrast) return serve_private_file( request, document.file, download_name=document.download_name, as_attachment=True, within=document.storage_prefix, ) api/routers/documents.py:346-350 if source == "rendered": document = owned_or_404(request, RenderedDocument.objects, pk) return serve_private_file( request, docum … ``` **Verification** (verdict: confirmed; severity after verification: Low). Line numbers are right: views.py:794-802 and api/routers/documents.py:346-350. The guarantee quoted from #663 is written in core/files.py about uploads ('an upload is kept under its owner's own'). It is not a promise made for every file-bearing model. The same gap exists for the other serve_private_file callers (jobs/page_views.py CapturedRenderingView, core/views_export.py export archives), so RenderedDocument is not unique. Remediation: pass within=f'documents/{document.owner_id}/' in both rendered branches, or add a storage_prefix property to RenderedDocument, and add a test beside the upload one. No attacker can create the precondition today. **Suggested remediation.** Give RenderedDocument the same `storage_prefix` property (`documents/{owner_id}/`) and pass `within=document.storage_prefix` from RenderedDownloadView and from the API's rendered branch, so a snapshot row is held to its owner's folder exactly as an upload is. Add the rendered case to tests/security/test_files.py beside the upload one. **Also covered by this issue.** - **Rendered-document download does not pin the file to the owner's storage prefix, unlike uploads (defence-in-depth gap)** (`src/postulo/api/routers/documents.py` (346-350); Informational). None on its own; a second mistake elsewhere is required. Recorded because the project treats this check as a rule for uploads and the sent CV/letter is the file it considers most sensitive. - **Export archive download does not pass the `within` owner-folder guard that uploads use** (`src/postulo/core/views_export.py` (86-94); Informational). One wrong file name on one row becomes another person's file. No path reaches it from a request today, so this is a hardening gap rather than an exploitable hole. - **Upload ownership guard `within=` is a raw string-prefix test that `..` segments in a stored name defeat** (`src/postulo/core/files.py` (98-113); Informational). If any row ever carries such a name, the #663 promise 'an upload is served only from its owner's folder' does not hold, and a file in another account's folder is handed over. The guard is the second line of defence behind the owner-scoped queryset. … - **The 'within' owner-folder guard on private files is a plain startswith and does not normalise '..' segments** (`src/postulo/core/files.py` (92-95); Informational). None today; the guard would not hold in the case it was written for if a traversal name ever reached the column.

tiagoagueda added the

bug

security

labels

2026-10-10 10:56:17 +00:00

tiagoagueda added this to the 0.6.0 milestone

2026-10-10 10:57:48 +00:00

tiagoagueda referenced this issue

2026-10-10 10:57:48 +00:00

PST-045: Owner scoping rests on each write path: several read helpers and generic relations take ids without an owner filter #774

tiagoagueda referenced this issue

2026-10-10 10:58:40 +00:00