Enforce the max file size against the bytes a chunked upload assembles

store() checks Setting::MaxFileSizeMb against the size the client declares
when it opens the session, and complete() re-checks the storage quota
against the real assembled byte count -- but nothing re-checked the size
limit itself. A client that declared a one-byte upload and then streamed
gigabytes of parts passed store()'s check and was never stopped, so the
configured limit (which store() applies to everyone, staff included) did
not hold for the resumable path that real uploads use.

Re-check the assembled byte count against MaxFileSizeMb in complete(),
cleaning up the assembled bytes and the session exactly as the quota
branch already does.
This commit is contained in:
denkfabrik-li
2026-08-25 20:32:13 +02:00
parent 6ab90aee79
commit 71d6b8937e
2 changed files with 51 additions and 0 deletions
@@ -249,6 +249,22 @@ class ChunkedUploadsController extends Controller
throw ValidationException::withMessages(['parts' => $exception->getMessage()]);
}
// store()'s size check ran against the client-declared, unverified
// size, so a small declared size would otherwise let an upload of any
// size through here. Re-check the real assembled byte count against
// the same limit store() applies to everyone. No File row exists yet
// at this point, so cleanup only needs to undo what assemble() wrote.
$maxMb = (int) $this->settings->get(Setting::MaxFileSizeMb);
if ($maxMb > 0 && $assembled['size'] > $maxMb * 1024 * 1024) {
Storage::disk($assembled['disk'])->delete($assembled['path']);
$session->delete();
throw ValidationException::withMessages([
'size' => __('This file exceeds the maximum allowed size of :max MB.', ['max' => (string) $maxMb]),
]);
}
// store()'s quota check used a client-declared, unverified size —
// re-check against the real assembled byte count before this
// becomes a File row. No File row exists yet at this point, so
@@ -152,6 +152,41 @@ test('a staff account without the upload permission cannot create sessions', fun
$this->postJson('/uploads', ['filename' => 'x.zip', 'size' => 10])->assertForbidden();
});
test('complete refuses a file whose real assembled size exceeds the limit, even when a tiny size was declared', function () {
$this->actingAs($this->admin);
// A 1 MB cap. The session is declared as a single byte, so it sails
// through store()'s check against the client-supplied size.
app(Settings::class)->set(Setting::MaxFileSizeMb, 1);
$sessionId = createSession(1, 'sneaky.zip');
// But the real parts stream ~1.5 MB.
$chunk = str_repeat('a', 800 * 1024);
putPart($sessionId, 1, $chunk)->assertOk();
putPart($sessionId, 2, $chunk)->assertOk();
$this->postJson("/uploads/{$sessionId}/complete")->assertStatus(422);
// Nothing is kept: no File row, the assembled bytes are removed, and the
// session is gone.
expect(File::query()->count())->toBe(0)
->and(UploadSession::query()->find($sessionId))->toBeNull()
->and(Storage::disk('files')->allFiles())->toBe([]);
});
test('complete still accepts a file at exactly the limit', function () {
$this->actingAs($this->admin);
app(Settings::class)->set(Setting::MaxFileSizeMb, 1);
$sessionId = createSession(1024 * 1024, 'exact.zip');
putPart($sessionId, 1, str_repeat('a', 1024 * 1024))->assertOk();
$response = $this->postJson("/uploads/{$sessionId}/complete")->assertOk();
expect(File::query()->findOrFail($response->json('file_id'))->size)->toBe(1024 * 1024);
});
test('a client with the upload permission can create a session and complete an upload', function () {
$client = User::factory()->client()->create();
grantChunkedUploadPermission($client);