mirror of
https://github.com/projectsend/projectsend.git
synced 2026-09-17 09:05:08 +00:00
6ad26bb61e
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
126 lines
4.2 KiB
PHP
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);
|
|
}
|
|
}
|