From 76a36386fceea2b102bb2b43b7e8516370b329ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20B=C3=B6hmer?= Date: Thu, 9 Jul 2026 19:33:11 +0200 Subject: [PATCH] Refactored size checks for attachments and check base64 upload length --- src/Controller/ToolsController.php | 2 +- src/Form/AttachmentFormType.php | 2 +- .../Attachments/AttachmentSubmitHandler.php | 36 ++++++++++++-- .../SystemSettings/AttachmentsSettings.php | 14 ------ .../AttachmentsSettingsTest.php | 47 ------------------- 5 files changed, 33 insertions(+), 68 deletions(-) delete mode 100644 tests/Settings/SystemSettings/AttachmentsSettingsTest.php diff --git a/src/Controller/ToolsController.php b/src/Controller/ToolsController.php index 76dffb4d..a92a863f 100644 --- a/src/Controller/ToolsController.php +++ b/src/Controller/ToolsController.php @@ -74,7 +74,7 @@ class ToolsController extends AbstractController 'detailed_error_pages' => $this->getParameter('partdb.error_pages.show_help'), 'error_page_admin_email' => $this->getParameter('partdb.error_pages.admin_email'), 'configured_max_file_size' => $settings->system->attachments->maxFileSize, - 'effective_max_file_size' => $attachmentSubmitHandler->getMaximumAllowedUploadSize(), + 'effective_max_file_size' => $attachmentSubmitHandler->getMaximumEffectiveUploadSize(), 'saml_enabled' => $this->getParameter('partdb.saml.enabled'), //PHP section diff --git a/src/Form/AttachmentFormType.php b/src/Form/AttachmentFormType.php index d9fe2cd2..2773a950 100644 --- a/src/Form/AttachmentFormType.php +++ b/src/Form/AttachmentFormType.php @@ -209,7 +209,7 @@ class AttachmentFormType extends AbstractType public function finishView(FormView $view, FormInterface $form, array $options): void { - $view->vars['max_upload_size'] = $this->submitHandler->getMaximumAllowedUploadSize(); + $view->vars['max_upload_size'] = $this->submitHandler->getMaximumEffectiveUploadSize(); } public function getBlockPrefix(): string diff --git a/src/Services/Attachments/AttachmentSubmitHandler.php b/src/Services/Attachments/AttachmentSubmitHandler.php index 2bc18c6e..5fe5cdfb 100644 --- a/src/Services/Attachments/AttachmentSubmitHandler.php +++ b/src/Services/Attachments/AttachmentSubmitHandler.php @@ -218,6 +218,14 @@ class AttachmentSubmitHandler //If no file was uploaded, but we have base64 encoded data, create a file from it if (!$file && $upload->data !== null) { + if (strlen($upload->data) > $this->getMaximumUserConfiguredUploadSize() * 4 / 3) { //Base64 encoding increases the size of the data by 4/3, so we have to check for that + throw new RuntimeException( + sprintf( + 'The given base64 data is too big! Maximum size is %.1f MB!', + $this->getMaximumUserConfiguredUploadSize() / 1000 / 1000 + )); + } + $file = new UploadedBase64EncodedFile(new Base64EncodedFile($upload->data), $upload->filename ?? 'base64'); } @@ -226,6 +234,15 @@ class AttachmentSubmitHandler //When a file is given then upload it, otherwise check if we need to download the URL if ($file instanceof UploadedFile) { + //Check the file size, to avoid uploading too big files. + //The file is not necessarily validated as it can also come from an Base64 source + if ($file->getSize() > $this->getMaximumUserConfiguredUploadSize()) { + throw new RuntimeException( + sprintf( + 'The uploaded file is too big! Maximum size is %.1f MB!', + $this->getMaximumUserConfiguredUploadSize() / 1000 / 1000 + )); + } $this->upload($attachment, $file, $secure_attachment); } elseif ($upload->downloadUrl && $attachment->hasExternal()) { @@ -402,7 +419,7 @@ class AttachmentSubmitHandler $fileHandler = fopen($tmp_path, 'wb'); $bytesDownloaded = 0; - $maxSize = $this->attachmentsSettings->getMaxFileSizeInMegabytes() * 1024 * 1024; //Convert to bytes + $maxSize = $this->getMaximumUserConfiguredUploadSize(); //We use the maximum user configured size here, PHPs limits dont apply //Write the downloaded data to file foreach ($this->httpClient->stream($response) as $chunk) { @@ -418,8 +435,8 @@ class AttachmentSubmitHandler throw new AttachmentDownloadException( sprintf( - 'The downloaded file is too big! Maximum size is %d MB!', - $this->settings->getMaxFileSizeInMegabytes() + 'The downloaded file is too big! Maximum size is %.1f MB!', + $maxSize / 1000 / 1000 )); } @@ -527,10 +544,10 @@ class AttachmentSubmitHandler } /* - * Returns the maximum allowed upload size in bytes. + * Returns the maximum effective upload size in bytes. * This is the minimum value of Part-DB max_file_size, and php.ini's post_max_size and upload_max_filesize. */ - public function getMaximumAllowedUploadSize(): int + public function getMaximumEffectiveUploadSize(): int { if ($this->max_upload_size_bytes) { return $this->max_upload_size_bytes; @@ -545,6 +562,15 @@ class AttachmentSubmitHandler return $this->max_upload_size_bytes; } + /** + * Returns the maximum user configured upload size in bytes. + * @return int + */ + public function getMaximumUserConfiguredUploadSize(): int + { + return $this->parseFileSizeString($this->settings->maxFileSize); + } + /** * Sanitizes the given SVG file, if the attachment is an internal SVG file. * @param Attachment $attachment diff --git a/src/Settings/SystemSettings/AttachmentsSettings.php b/src/Settings/SystemSettings/AttachmentsSettings.php index 235b7b85..5aa3f91d 100644 --- a/src/Settings/SystemSettings/AttachmentsSettings.php +++ b/src/Settings/SystemSettings/AttachmentsSettings.php @@ -65,18 +65,4 @@ class AttachmentsSettings envVar: "bool:ATTACHMENT_SHOW_HTML_FILES", envVarMode: EnvVarMode::OVERWRITE )] public bool $showHTMLAttachments = false; - - public function getMaxFileSizeInMegabytes(): int - { - $size = $this->maxFileSize; - $unit = strtoupper(substr($size, -1)); - $value = (int) rtrim($size, 'KMG'); - - return match ($unit) { - 'K' => (int) ceil($value / 1024), - 'M' => $value, - 'G' => $value * 1024, - default => $value, // No unit specified, assume megabytes - }; - } } diff --git a/tests/Settings/SystemSettings/AttachmentsSettingsTest.php b/tests/Settings/SystemSettings/AttachmentsSettingsTest.php deleted file mode 100644 index 16acc13c..00000000 --- a/tests/Settings/SystemSettings/AttachmentsSettingsTest.php +++ /dev/null @@ -1,47 +0,0 @@ -. - */ - -namespace App\Tests\Settings\SystemSettings; - -use App\Settings\SystemSettings\AttachmentsSettings; -use App\Tests\SettingsTestHelper; -use PHPUnit\Framework\TestCase; - -class AttachmentsSettingsTest extends TestCase -{ - - public function testGetMaxFileSizeInMegabytes(): void - { - $settings = SettingsTestHelper::createSettingsDummy(AttachmentsSettings::class); - - $settings->maxFileSize = '100M'; - $this->assertEquals(100, $settings->getMaxFileSizeInMegabytes()); - - $settings->maxFileSize = '1G'; - $this->assertEquals(1024, $settings->getMaxFileSizeInMegabytes()); - - //We round up to the next megabyte if the file size is smaller than 1 MB - $settings->maxFileSize = '500K'; - $this->assertEquals(1, $settings->getMaxFileSizeInMegabytes()); - - $settings->maxFileSize = '13.5M'; - $this->assertEquals(13, $settings->getMaxFileSizeInMegabytes()); - } -}