From 71d6b8937ea90649113afbc1578f5bde28fe45ea Mon Sep 17 00:00:00 2001 From: denkfabrik-li <274324701+denkfabrik-li@users.noreply.github.com> Date: Tue, 25 Aug 2026 20:32:13 +0200 Subject: [PATCH] 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. --- .../Controllers/ChunkedUploadsController.php | 16 +++++++++ tests/Feature/Files/ChunkedUploadsTest.php | 35 +++++++++++++++++++ 2 files changed, 51 insertions(+) diff --git a/app/Modules/Files/Http/Controllers/ChunkedUploadsController.php b/app/Modules/Files/Http/Controllers/ChunkedUploadsController.php index 1f376bf3..e3421213 100644 --- a/app/Modules/Files/Http/Controllers/ChunkedUploadsController.php +++ b/app/Modules/Files/Http/Controllers/ChunkedUploadsController.php @@ -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 diff --git a/tests/Feature/Files/ChunkedUploadsTest.php b/tests/Feature/Files/ChunkedUploadsTest.php index 1959159d..5669413e 100644 --- a/tests/Feature/Files/ChunkedUploadsTest.php +++ b/tests/Feature/Files/ChunkedUploadsTest.php @@ -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);