Summary

POST /upload uses the request-supplied resumableFilename directly as a key into the shared temporary directory, without the character filter it applies to resumableIdentifier and without namespacing it per user. A valid user holding the upload permission can read any file in that directory into their own home directory, append to it, and delete it.

This reproduces the cross-user batch-archive disclosure of GHSA-f74m-x83r-c4v4 through a different route — it still works with the fix for that advisory (PR #597) applied — and additionally allows deleting the login rate-limit lockfile.

Reported with explicit preconditions and bounded impact, as before: the primitive is confined to the temporary directory, not the whole filesystem.

Details

- backend/Controllers/UploadController.php:61 — $filename = $request->input('resumableFilename', 'file') is not passed through the pregreplace('/[^0-9a-zA-Z]/', '', ...) that line 66 applies to resumableIdentifier. - Line 88 builds $prefix namespaced by the sanitized username, but $filename reaches the temporary storage at lines 118, 121 and 125 without that prefix. - Line 115 — if ($chunkssize >= $totalsize) is entered whenever resumableTotalSize is 0. - Lines 116-119 — the assembly loop for ($i = 1; $i <= $totalchunks; ++$i) does not execute when resumableTotalChunks is 0, so nothing is written to $filename. - Line 121 — readStream($filename) therefore reads the pre-existing temporary file of that exact name. - Line 122 — its contents are stored into the requester's own home directory. - Line 125 — remove($filename) deletes it.

Path traversal outside the temporary directory is prevented by Tmpfs::sanitizeFilename(), so the primitive is bounded to that directory — but every user's transient objects share it.

PoC

Preconditions: a multi-user deployment with distinct per-user homedirs. The attacker is a valid low-privileged user holding upload (a first-class permission in the admin UI, and present in the shipped users.json.blank admin set).

1. Victim creates a batch archive via POST /batchdownload; the JSON response returns the archive id. 2. Attacker sends one POST /upload with resumableFilename=<victim's id>, resumableTotalChunks=0, resumableTotalSize=0, resumableRelativePath=/, and any small valid file part. 3. Response is {"data":"Stored"}. The victim's archive is now byte-identical in the attacker's home directory, retrievable through the ordinary GET /download. The victim's temporary archive has been deleted.

Verified end to end with a PHPUnit test against current master; the archive contents matched byte for byte. Test file available on request.

Impact

Missing authorization (CWE-862) on temporary storage objects.

- Cross-user disclosure of any transient object whose name the attacker can learn or predict. This includes batch archives — so GHSA-f74m-x83r-c4v4's impact remains reachable even with its fix applied — and the plaintext per-file copies that ZipArchiver::addFileFromStorage() writes while an archive is being built (those still use uniqid(), which is timestamp-derived). - Arbitrary delete within the temporary directory. AuthController::login() stores the login rate-limit counter as md5($ip).'.lock' in that same directory (backend/Controllers/AuthController.php:36, checked at line 44, written at line 58). An attacker can compute their own lockfile name and delete it, defeating the lockout. - Arbitrary append: with resumableTotalChunks >= 1, line 118 write($filename, ..., true) appends attacker-controlled bytes to an arbitrary existing temporary file.

Remediation

Do not use the request-supplied filename as a storage key. Namespace the assembly file per user — for example reuse the existing $prefix — and carry the display filename separately for the storage->store() call.

Note that simply applying the [^0-9a-zA-Z] filter to resumableFilename would corrupt legitimate uploads (sample.txt becomes sampletxt), because $final['filename'] is what becomes the stored name. The fix needs the namespacing, not just the filter.

Summary GET /batchdownload streams a batch ZIP to any requester who supplies its uniqid, without checking that the current user/session created it. In deployments where users have separate home directories, a valid low-privileged user who obtains an active archive id can download another user's batch archive, disclosing files the per-user boundary otherwise protects. Reported with explicit preconditions and bounded impact — not a universal "any user steals any file" issue.

Details The archive id is generated with plain uniqid() and no ownership metadata is bound to it before streaming.

- backend/Services/Archiver/Adapters/ZipArchiver.php:44 — createArchive() uses plain uniqid(). - backend/Controllers/DownloadController.php:144-145 — batchDownloadStart() reads the tmpfs file by the request-supplied uniqid after only a charset pregreplace(). - backend/Controllers/DownloadController.php:158 — the tmpfs archive is removed only after streaming. - backend/Controllers/routes.php:207-226 — both batch endpoints are exposed to normal roles with read, download, batchdownload.

No user id, session id, home directory, or creator check is applied before streaming. The code path is unchanged since commit 3f03a40 (2022-05-31) and is present in the latest release v7.14.3.

PoC Preconditions: a multi-user deployment with distinct per-user homedirs (set per-user in users.json, or privaterepos => true on the WordPress/LDAP adapter). The shipped sample config (admin/guest both at /, guest with no perms) does not create this boundary on its own. Attacker is a valid user with read, download, batchdownload.

1. User John (homedir /john) creates secret.txt, signs in, and creates a batch archive via POST /batchdownload. The JSON response returns the archive uniqid. 2. User Jane (homedir /jane, with batchdownload) signs in normally. 3. Jane's direct GET /download of /secret.txt returns 302 — the normal home-directory boundary blocks the file. 4. Jane sends GET /batchdownload?uniqid=<John's id> and receives a valid ZIP containing secret.txt with John's content.

A PHPUnit proof with two real sessions reproduces this end to end (OK, 3 tests / 29 assertions), plus a lifecycle test (OK, 2 tests / 9 assertions). Full test files, run logs, and a real Apache access-log replay demo are available on request.

Lifetime / acquisition: a normal completed download removes the archive at the end of streaming, limiting the window. An aborted/failed download leaves the archive in tmpfs (cleanup never reached). The frontend issues the download as a top-level navigation (window.open('.../batchdownload&uniqid='+ret.uniqid, 'blank')), so the id appears in standard web access logs. That access-log path only adds meaningful impact when log visibility is decoupled from host filesystem access (e.g. a log-aggregation/shared-hosting/downstream-sink reader) and the archive is still alive; if the reader is ops/root on the same host they can already read the storage tree directly. The uniqid() token is timestamp-derived (1,000,000-value space per known second) — included to justify a CSPRNG, not presented as a practical universal brute force.

Impact Missing authorization (CWE-862) on a temporary download object, leading to cross-user file disclosure across the per-user home-directory boundary in configured multi-user deployments. Secondary effect: a successful replay deletes the victim's pending archive (the GET removes the tmpfs file after streaming), denying the victim their own download if not yet consumed.

Remediation: generate ids with bin2hex(randombytes(32)); store creator user/session + home dir + expiry per archive id; verify the requester matches before streaming and return 403/404 on mismatch or unknown/expired id; delete metadata with the tmpfs file.