Files
ignacionelson 6ad26bb61e Hold an upload to the size it said it was sending
Reported by @ry2811 as GHSA-6jh6-gvj5-pv8v.

A resumable upload declares its size, and that declaration is what store()
weighs against the maximum file size and the client's storage quota. Only
the assembled file was ever held to it. The parts in between were bounded
one request at a time and never added up, so a client could declare one
byte and then stream parts: ten thousand part numbers at twice a 20 MB
part is about 400 GB, per session, and the number of sessions was not
bounded either. None of it counted against anything, because nothing
becomes a File row until the upload completes and ClientStorageUsage sums
File rows. A client with a 1 MB quota could fill the volume and repeat.

putPart()'s own comment described this defect and treated the per-part cap
as the answer to it: "without a cap here the exposure is a day's worth of
disk". A cap on one request bounds one request. The exposure was a day's
worth of disk multiplied by however many requests somebody cared to make.

Three limits, and each one exists because the other two do not cover it.

A session may not stage more than it declared. The room for a part is
claimed before the body is read — a body's length is not known until it
has arrived, and by then it is on the disk being protected — and the write
is then capped at exactly what was claimed, so an over-long body is cut
off mid-stream as it always was, against a smaller number. The claim is a
read and a conditional update under a per-session lock, the same shape
complete() already uses: the protocol sends parts in parallel and how many
is the client's choice, so an unlocked read lets every part in flight
claim the same room, while an atomic claim alone refuses the honest
parallel upload instead. Whatever the part really weighs is settled back
afterwards, in a finally, or a client's own retries would exhaust a
session with room to spare.

Open sessions count against the quota at the size they declared. A quota
measured against finished files alone is spent twice by opening sessions
one after another — each is told there is room, because the ones before it
have not finished. The cost is that an abandoned transfer holds its share
until it is cancelled or swept, so the sweeper now runs hourly rather than
daily: that gap is now somebody unable to upload, which it was not before.

And a cap on open sessions, because for anyone with no quota to spend —
staff, and clients on an installation that sets none — the session count
is the only thing between a declared size and any multiple of it.

Four tests fail on the unfixed code, and three existing ones had to change:
they declared a tiny size and sent a large part deliberately, to reach the
re-checks at complete(). That route is now closed at putPart(), so they
reach those re-checks the way a real install would instead — the file-size
limit or the quota moving while a long transfer is running, which is the
reason complete() re-asks rather than trusting what store() decided.

The staged-byte total is BIGINT UNSIGNED, and the suite runs SQLite, which
has no unsigned integers. The first version of the bounds read
`staged_bytes + :delta BETWEEN 0 AND size` and raised SQLSTATE 22003 on
MySQL for any refund — in the comparison, so the bound written to prevent
the underflow was the statement that underflowed. Every SQLite test passed
on it. Both bounds are now arranged so the column is never inside a
subtraction, and UploadSessionStagedBytesMysqlTest skips loudly unless the
connection is MySQL. Verified against 8.4, as was the report itself: three
sessions declaring one byte each put 6 MB on the volume of a client with a
1 MB quota before, and nothing at all after.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CNFU55Tkq6MuEQ73nbbBRx
2026-09-11 12:05:41 -03:00

126 lines
4.2 KiB
PHP

<?php
declare(strict_types=1);
namespace App\Modules\Files\Uploads;
use App\Models\User;
use App\Modules\Files\Models\Folder;
use Illuminate\Database\Eloquent\Concerns\HasUuids;
use Illuminate\Database\Eloquent\Model;
use Illuminate\Database\Eloquent\Relations\BelongsTo;
use Illuminate\Support\Facades\DB;
/**
* A resumable chunked upload in progress. Parts live on disk under the
* session's temp directory — the filesystem is the part ledger.
*
* @property string $id
* @property int $user_id
* @property int|null $folder_id
* @property int|null $previous_file_id
* @property string $original_name
* @property int $size
* @property int $staged_bytes
* @property string|null $mime_type
* @property string|null $description
* @property string $status
* @property-read User $user
*/
class UploadSession extends Model
{
use HasUuids;
public const STATUS_OPEN = 'open';
protected $guarded = [];
/**
* @return BelongsTo<User, $this>
*/
public function user(): BelongsTo
{
return $this->belongsTo(User::class);
}
/**
* @return BelongsTo<Folder, $this>
*/
public function folder(): BelongsTo
{
return $this->belongsTo(Folder::class);
}
public function isOwnedBy(User $user): bool
{
return $this->user_id === $user->id;
}
/**
* Claim room on the temporary volume for a part that is about to
* arrive, returning false if the session has no room left.
*
* The claim is made before the bytes are read, and it is one
* statement, because neither weaker version holds. Checking the part
* directory and then writing leaves a gap that every other part
* currently in flight fits through — and the protocol sends parts in
* parallel, so the number of them is the caller's choice, not ours.
* Charging the real size afterwards is the same gap by another name.
*
* $replacing is what the part number already holds, since re-sending a
* part overwrites it rather than adding to it. That is an ordinary
* resume, not an attack.
*
* The ceiling is the size the session declared. A client cannot stage
* more than it said it was sending, which is the invariant the whole
* fix rests on: store() has already measured that declaration against
* the file-size limit and the storage quota, so bounding staged bytes
* by it puts temporary bytes under the same limits as stored ones.
*/
public function reserveStaged(int $bytes, int $replacing = 0): bool
{
$delta = $bytes - $replacing;
$query = static::query()->whereKey($this->getKey());
// Both bounds are rearranged so that the column is never part of a
// subtraction. `staged_bytes + :delta BETWEEN 0 AND size` reads
// naturally and is wrong: staged_bytes is BIGINT UNSIGNED, and on
// MySQL a negative delta makes that expression underflow and raise
// SQLSTATE 22003 — in the comparison, before any row is chosen, so
// the bound meant to prevent it is the thing that trips over it.
// SQLite has no unsigned integers, so the suite cannot see this at
// all; UploadSessionStagedBytesMysqlTest is what covers it.
if ($delta >= 0) {
if ($delta > $this->size) {
return false;
}
$query->where('staged_bytes', '<=', $this->size - $delta);
} else {
// Never give back more than is held.
$query->where('staged_bytes', '>=', -$delta);
}
return $query->update(['staged_bytes' => DB::raw(sprintf('staged_bytes + (%d)', $delta))]) === 1;
}
/**
* Replace a reservation with what the part actually weighs.
*
* Always called, whatever happened to the part: a body shorter than
* its Content-Length, a client that hung up mid-transfer, a part
* refused for being too long and deleted. Whatever is on disk now is
* the truth, and the difference goes back to the session — otherwise
* a client's own retries would slowly exhaust their room.
*/
public function settleStaged(int $reserved, int $actual): void
{
if ($reserved === $actual) {
return;
}
$this->reserveStaged($actual, $reserved);
}
}