FileGator: Missing authorization in POST /upload allows cross-user read and deletion of temporary files

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 — $file_name = $request->input('resumableFilename', 'file') is not passed through the preg_replace('/[^0-9a-zA-Z_]/', '', ...) that line 66 applies to resumableIdentifier.
• Line 88 builds $prefix namespaced by the sanitized username, but $file_name reaches the temporary storage at lines 118, 121 and 125 without that prefix.
• Line 115 — if ($chunks_size >= $total_size) is entered whenever resumableTotalSize is 0.
• Lines 116-119 — the assembly loop for ($i = 1; $i <= $total_chunks; ++$i) does not execute when resumableTotalChunks is 0, so nothing is written to $file_name.
• Line 121 — readStream($file_name) 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($file_name) 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($file_name, ..., 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.