diff --git a/docs/configuration.md b/docs/configuration.md index 709c39b3..b4c3d747 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -86,6 +86,9 @@ bundled with Part-DB. Set `DATABASE_MYSQL_SSL_VERIFY_CERT` if you want to accept * `ATTACHMENT_DOWNLOAD_BY_DEFAULT`: When this is set to 1, the "download external file" checkbox is checked by default when adding a new attachment. Otherwise, it is unchecked by default. Use this if you wanna download all attachments locally by default. Attachment download is only possible, when `ALLOW_ATTACHMENT_DOWNLOADS` is set to 1. +* `ATTACHMENT_SHOW_HTML_FILES`: 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. When set to 0, HTML files are rendered as plain text. * `USE_GRAVATAR`: Set to `1` to use [gravatar.com](https://gravatar.com/) images for user avatars (as long as they have not set their own picture). The users browsers have to download the pictures from a third-party (gravatar) server, so this might be a privacy risk. diff --git a/src/Controller/AttachmentFileController.php b/src/Controller/AttachmentFileController.php index 81369e12..01aeab11 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,31 +42,56 @@ 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 allow-downloads allow-modals;"); + + //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); + $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; } @@ -74,7 +100,35 @@ 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); + + $response = $this->forbidHTMLContentType($response); + + //Set header content disposition, so that the file will be downloaded + $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; + } + + private function checkPermissions(Attachment $attachment): void { $this->denyAccessUnlessGranted('read', $attachment); @@ -86,17 +140,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/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/src/Settings/SystemSettings/AttachmentsSettings.php b/src/Settings/SystemSettings/AttachmentsSettings.php index 6d15c639..5aa3f91d 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_FILES", 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..389ebc2e --- /dev/null +++ b/templates/attachments/html_sandbox.html.twig @@ -0,0 +1,78 @@ + + + + + + + + {# The content block is already escaped. so we must not escape it again. #} + {% trans %}attachment.sandbox.title{% endtrans %}: {{ attachment.filename }} + + + + +{% 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.as_plain_text{% endtrans %} + {% trans%}attachment.sandbox.back_to_partdb{% endtrans %} + +
+
+ + + + +
+ +{% endblock %} + + + 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()); + } } diff --git a/translations/messages.en.xlf b/translations/messages.en.xlf index 356aa89a..7bf613ae 100644 --- a/translations/messages.en.xlf +++ b/translations/messages.en.xlf @@ -12593,5 +12593,41 @@ 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. + + + + + attachment.sandbox.title + HTML [Attachment] + + + + + attachment.sandbox.as_plain_text + View as plain text + +