From 63dd344c02e3abeec071973fd223dae679f971dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20B=C3=B6hmer?= Date: Tue, 24 Feb 2026 22:27:33 +0100 Subject: [PATCH 1/7] Added basic functionality for an HTML sandbox for relative safely rendering HTML attachments Fixed #1150 --- src/Controller/AttachmentFileController.php | 79 +++++++++++++------ .../SystemSettings/AttachmentsSettings.php | 9 ++- templates/attachments/html_sandbox.html.twig | 75 ++++++++++++++++++ translations/messages.en.xlf | 24 ++++++ 4 files changed, 161 insertions(+), 26 deletions(-) create mode 100644 templates/attachments/html_sandbox.html.twig diff --git a/src/Controller/AttachmentFileController.php b/src/Controller/AttachmentFileController.php index 81369e12..c16c1e85 100644 --- a/src/Controller/AttachmentFileController.php +++ b/src/Controller/AttachmentFileController.php @@ -30,6 +30,7 @@ use App\Form\Filters\AttachmentFilterType; use App\Services\Attachments\AttachmentManager; use App\Services\Trees\NodesListBuilder; use App\Settings\BehaviorSettings\TableSettings; +use App\Settings\SystemSettings\AttachmentsSettings; use Omines\DataTablesBundle\DataTableFactory; use RuntimeException; use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; @@ -41,27 +42,50 @@ use Symfony\Component\Routing\Attribute\Route; class AttachmentFileController extends AbstractController { + + public function __construct(private readonly AttachmentManager $helper) + { + + } + + #[Route(path: '/attachment/{id}/sandbox', name: 'attachment_html_sandbox')] + public function htmlSandbox(Attachment $attachment, AttachmentsSettings $attachmentsSettings): Response + { + //Check if the sandbox is enabled in the settings, as it can be a security risk if used without proper precautions, so it should be opt-in + if (!$attachmentsSettings->showHTMLAttachments) { + throw $this->createAccessDeniedException('The HTML sandbox for attachments is disabled in the settings, as it can be a security risk if used without proper precautions. Please enable it in the settings if you want to use it.'); + } + + $this->checkPermissions($attachment); + + $file_path = $this->helper->toAbsoluteInternalFilePath($attachment); + + $attachmentContent = file_get_contents($file_path); + + $response = $this->render('attachments/html_sandbox.html.twig', [ + 'attachment' => $attachment, + 'content' => $attachmentContent, + ]); + + //Set an CSP that allows to run inline scripts, styles and images from external ressources, but does not allow any connections or others. + //Also set the sandbox CSP directive with only "allow-script" to run basic scripts + $response->headers->set('Content-Security-Policy', "default-src 'none'; script-src 'unsafe-inline'; style-src 'unsafe-inline'; img-src data:; sandbox allow-scripts;"); + + //Forbid to embed the attachment render page in an iframe to prevent clickjacking, as it is not used anywhere else for now + $response->headers->set('X-Frame-Options', 'DENY'); + + return $response; + } + /** * Download the selected attachment. */ #[Route(path: '/attachment/{id}/download', name: 'attachment_download')] - public function download(Attachment $attachment, AttachmentManager $helper): BinaryFileResponse + public function download(Attachment $attachment): BinaryFileResponse { - $this->denyAccessUnlessGranted('read', $attachment); + $this->checkPermissions($attachment); - if ($attachment->isSecure()) { - $this->denyAccessUnlessGranted('show_private', $attachment); - } - - if (!$attachment->hasInternal()) { - throw $this->createNotFoundException('The file for this attachment is external and not stored locally!'); - } - - if (!$helper->isInternalFileExisting($attachment)) { - throw $this->createNotFoundException('The file associated with the attachment is not existing!'); - } - - $file_path = $helper->toAbsoluteInternalFilePath($attachment); + $file_path = $this->helper->toAbsoluteInternalFilePath($attachment); $response = new BinaryFileResponse($file_path); //Set header content disposition, so that the file will be downloaded @@ -74,7 +98,20 @@ class AttachmentFileController extends AbstractController * View the attachment. */ #[Route(path: '/attachment/{id}/view', name: 'attachment_view')] - public function view(Attachment $attachment, AttachmentManager $helper): BinaryFileResponse + public function view(Attachment $attachment): BinaryFileResponse + { + $this->checkPermissions($attachment); + + $file_path = $this->helper->toAbsoluteInternalFilePath($attachment); + $response = new BinaryFileResponse($file_path); + + //Set header content disposition, so that the file will be downloaded + $response->setContentDisposition(ResponseHeaderBag::DISPOSITION_INLINE); + + return $response; + } + + private function checkPermissions(Attachment $attachment): void { $this->denyAccessUnlessGranted('read', $attachment); @@ -86,17 +123,9 @@ class AttachmentFileController extends AbstractController throw $this->createNotFoundException('The file for this attachment is external and not stored locally!'); } - if (!$helper->isInternalFileExisting($attachment)) { + if (!$this->helper->isInternalFileExisting($attachment)) { throw $this->createNotFoundException('The file associated with the attachment is not existing!'); } - - $file_path = $helper->toAbsoluteInternalFilePath($attachment); - $response = new BinaryFileResponse($file_path); - - //Set header content disposition, so that the file will be downloaded - $response->setContentDisposition(ResponseHeaderBag::DISPOSITION_INLINE); - - return $response; } #[Route(path: '/attachment/list', name: 'attachment_list')] diff --git a/src/Settings/SystemSettings/AttachmentsSettings.php b/src/Settings/SystemSettings/AttachmentsSettings.php index 6d15c639..2a682b11 100644 --- a/src/Settings/SystemSettings/AttachmentsSettings.php +++ b/src/Settings/SystemSettings/AttachmentsSettings.php @@ -58,4 +58,11 @@ class AttachmentsSettings envVar: "bool:ATTACHMENT_DOWNLOAD_BY_DEFAULT", envVarMode: EnvVarMode::OVERWRITE )] public bool $downloadByDefault = false; -} \ No newline at end of file + + #[SettingsParameter( + label: new TM("settings.system.attachments.showHTMLAttachments"), + description: new TM("settings.system.attachments.showHTMLAttachments.help"), + envVar: "bool:ATTACHMENT_SHOW_HTML", envVarMode: EnvVarMode::OVERWRITE + )] + public bool $showHTMLAttachments = false; +} diff --git a/templates/attachments/html_sandbox.html.twig b/templates/attachments/html_sandbox.html.twig new file mode 100644 index 00000000..6706da7d --- /dev/null +++ b/templates/attachments/html_sandbox.html.twig @@ -0,0 +1,75 @@ + + + + + + {# The content block is already escaped. so we must not escape it again. #} + + + + + +{% block body %} + {# We have a fullscreen iframe, with an warning on top #} + +
+ +
+
+ ⚠️ {% trans%}attachment.sandbox.warning{% endtrans %} + +
+ + {% trans%}[Attachment]{% endtrans%}: {{ attachment.name }} / {{ attachment.filename ?? "" }} ({% trans%}id.label{% endtrans %}: {{ attachment.id }}) + {% trans%}attachment.sandbox.back_to_partdb{% endtrans %} + +
+
+ + + + +
+ +{% endblock %} + + + diff --git a/translations/messages.en.xlf b/translations/messages.en.xlf index 356aa89a..e8475aab 100644 --- a/translations/messages.en.xlf +++ b/translations/messages.en.xlf @@ -12593,5 +12593,29 @@ Buerklin-API Authentication server: When selected, more details will be fetched from canopy when creating a part. This causes an additional API request, but gives product bullet points and category info. + + + attachment.sandbox.warning + WARNING: You are viewing an user uploaded attachment. This is untrusted content. Proceed with care. + + + + + attachment.sandbox.back_to_partdb + Back to Part-DB + + + + + settings.system.attachments.showHTMLAttachments + Show uploaded HTML file attachments (sandboxed) + + + + + settings.system.attachments.showHTMLAttachments.help + ⚠️ When enabled, user uploaded HTML attachments can be viewed directly in the browser. Many potential malicious functions are restricted, still this is a potential security risk and should only be enabled, if you trust the users who can upload files. + + From 4a5cc454ce49c4c21db604a13697ed9217092e5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20B=C3=B6hmer?= Date: Tue, 24 Feb 2026 22:40:23 +0100 Subject: [PATCH 2/7] Show HTML files in the HTML sandbox if enabled --- src/Entity/Attachments/Attachment.php | 16 +++++++++++ .../Attachments/AttachmentURLGenerator.php | 7 ++++- tests/Entity/Attachments/AttachmentTest.php | 28 +++++++++++++++++++ 3 files changed, 50 insertions(+), 1 deletion(-) diff --git a/src/Entity/Attachments/Attachment.php b/src/Entity/Attachments/Attachment.php index d4b15ac7..e0d1bd9d 100644 --- a/src/Entity/Attachments/Attachment.php +++ b/src/Entity/Attachments/Attachment.php @@ -296,6 +296,22 @@ abstract class Attachment extends AbstractNamedDBElement return in_array(strtolower($extension), static::MODEL_EXTS, true); } + /** + * Returns true if this is a locally stored HTML file, which can be shown by the sandbox viewer. + * This is the case if we have an internal path with a html extension. + * @return bool + */ + public function isLocalHTMLFile(): bool + { + if($this->hasInternal()){ + + $extension = pathinfo($this->getFilename(), PATHINFO_EXTENSION); + + return in_array(strtolower($extension), ['html', 'htm'], true); + } + return false; + } + /** * Checks if this attachment has a path to an external file * diff --git a/src/Services/Attachments/AttachmentURLGenerator.php b/src/Services/Attachments/AttachmentURLGenerator.php index e505408f..89856650 100644 --- a/src/Services/Attachments/AttachmentURLGenerator.php +++ b/src/Services/Attachments/AttachmentURLGenerator.php @@ -22,6 +22,7 @@ declare(strict_types=1); namespace App\Services\Attachments; +use App\Settings\SystemSettings\AttachmentsSettings; use Imagine\Exception\RuntimeException; use App\Entity\Attachments\Attachment; use InvalidArgumentException; @@ -40,7 +41,7 @@ class AttachmentURLGenerator public function __construct(protected Packages $assets, protected AttachmentPathResolver $pathResolver, protected UrlGeneratorInterface $urlGenerator, protected AttachmentManager $attachmentHelper, - protected CacheManager $thumbnailManager, protected LoggerInterface $logger) + protected CacheManager $thumbnailManager, protected LoggerInterface $logger, private readonly AttachmentsSettings $attachmentsSettings) { //Determine a normalized path to the public folder (assets are relative to this folder) $this->public_path = $this->pathResolver->parameterToAbsolutePath('public'); @@ -99,6 +100,10 @@ class AttachmentURLGenerator return null; } + if ($this->attachmentsSettings->showHTMLAttachments && $attachment->isLocalHTMLFile()) { + return $this->urlGenerator->generate('attachment_html_sandbox', ['id' => $attachment->getID()]); + } + $asset_path = $this->absolutePathToAssetPath($absolute_path); //If path is not relative to public path or marked as secure, serve it via controller if (null === $asset_path || $attachment->isSecure()) { diff --git a/tests/Entity/Attachments/AttachmentTest.php b/tests/Entity/Attachments/AttachmentTest.php index ca55424c..9e912b97 100644 --- a/tests/Entity/Attachments/AttachmentTest.php +++ b/tests/Entity/Attachments/AttachmentTest.php @@ -279,4 +279,32 @@ final class AttachmentTest extends TestCase $reflection_property->setAccessible(true); $reflection_property->setValue($object, $value); } + + public function testIsLocalHTMLFile(): void + { + $attachment = new PartAttachment(); + + $attachment->setExternalPath('https://google.de'); + $this->assertFalse($attachment->isLocalHTMLFile()); + + $attachment->setExternalPath('https://google.de/test.html'); + $this->assertFalse($attachment->isLocalHTMLFile()); + + $attachment->setInternalPath('%MEDIA%/test.html'); + $this->assertTrue($attachment->isLocalHTMLFile()); + + $attachment->setInternalPath('%MEDIA%/test.htm'); + $this->assertTrue($attachment->isLocalHTMLFile()); + + $attachment->setInternalPath('%MEDIA%/test.txt'); + $this->assertFalse($attachment->isLocalHTMLFile()); + + //It works however, if the file is stored as txt, and the internal filename ends with .html + $attachment->setInternalPath('%MEDIA%/test.txt'); + $this->setProtectedProperty($attachment, 'original_filename', 'test.html'); + $this->assertTrue($attachment->isLocalHTMLFile()); + + $this->setProtectedProperty($attachment, 'original_filename', 'test.htm'); + $this->assertTrue($attachment->isLocalHTMLFile()); + } } From a1fd3199d67b38deb13d9f532ba36219a74d7d22 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20B=C3=B6hmer?= Date: Tue, 24 Feb 2026 22:48:18 +0100 Subject: [PATCH 3/7] Render HTML as plain text via attachment_view controller This makes it consistent with the public paths and ensures all HTML is only rendered in our sandbox --- src/Controller/AttachmentFileController.php | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/src/Controller/AttachmentFileController.php b/src/Controller/AttachmentFileController.php index c16c1e85..278bcf6e 100644 --- a/src/Controller/AttachmentFileController.php +++ b/src/Controller/AttachmentFileController.php @@ -88,8 +88,10 @@ class AttachmentFileController extends AbstractController $file_path = $this->helper->toAbsoluteInternalFilePath($attachment); $response = new BinaryFileResponse($file_path); + $response = $this->forbidHTMLContentType($response); + //Set header content disposition, so that the file will be downloaded - $response->setContentDisposition(ResponseHeaderBag::DISPOSITION_ATTACHMENT); + $response->setContentDisposition(ResponseHeaderBag::DISPOSITION_ATTACHMENT, $attachment->getFilename()); return $response; } @@ -105,8 +107,23 @@ class AttachmentFileController extends AbstractController $file_path = $this->helper->toAbsoluteInternalFilePath($attachment); $response = new BinaryFileResponse($file_path); + $response = $this->forbidHTMLContentType($response); + //Set header content disposition, so that the file will be downloaded - $response->setContentDisposition(ResponseHeaderBag::DISPOSITION_INLINE); + $response->setContentDisposition(ResponseHeaderBag::DISPOSITION_INLINE, $attachment->getFilename()); + + return $response; + } + + private function forbidHTMLContentType(BinaryFileResponse $response): BinaryFileResponse + { + $mimeType = $response->getFile()->getMimeType(); + + if ($mimeType === 'text/html') { + $mimeType = 'text/plain'; + } + + $response->headers->set('Content-Type', $mimeType); return $response; } From 628f794b3708f85db68e38fd61d746ee88d9ebca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20B=C3=B6hmer?= Date: Tue, 24 Feb 2026 22:53:50 +0100 Subject: [PATCH 4/7] Improved HTML sandbox page --- templates/attachments/html_sandbox.html.twig | 7 +++++-- translations/messages.en.xlf | 12 ++++++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/templates/attachments/html_sandbox.html.twig b/templates/attachments/html_sandbox.html.twig index 6706da7d..11367bdf 100644 --- a/templates/attachments/html_sandbox.html.twig +++ b/templates/attachments/html_sandbox.html.twig @@ -4,8 +4,10 @@ + + {# The content block is already escaped. so we must not escape it again. #} - + {% trans %}attachment.sandbox.title{% endtrans %}: {{ attachment.filename }}