diff --git a/webapp/composer.json b/webapp/composer.json index 5d8b901bc8..760cb5e441 100644 --- a/webapp/composer.json +++ b/webapp/composer.json @@ -63,6 +63,7 @@ "doctrine/doctrine-migrations-bundle": "^3.2", "doctrine/orm": "^2.14", "eligrey/filesaver": "2.*", + "enshrined/svg-sanitize": "^0.21.0", "fortawesome/font-awesome": "6.*", "friendsofsymfony/rest-bundle": "^3.5", "ircmaxell/password-compat": "*", diff --git a/webapp/composer.lock b/webapp/composer.lock index e24711e652..24485370c8 100644 --- a/webapp/composer.lock +++ b/webapp/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "6401b4bdcb1db7464a2e9e4bcbf894b2", + "content-hash": "106fc22e21c1d0fbfb208159951d1736", "packages": [ { "name": "apalfrey/select2-bootstrap-5-theme", @@ -1867,6 +1867,51 @@ }, "type": "library" }, + { + "name": "enshrined/svg-sanitize", + "version": "0.21.0", + "source": { + "type": "git", + "url": "https://github.com/darylldoyle/svg-sanitizer.git", + "reference": "5e477468fac5c5ce933dce53af3e8e4e58dcccc9" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/darylldoyle/svg-sanitizer/zipball/5e477468fac5c5ce933dce53af3e8e4e58dcccc9", + "reference": "5e477468fac5c5ce933dce53af3e8e4e58dcccc9", + "shasum": "" + }, + "require": { + "ext-dom": "*", + "ext-libxml": "*", + "php": "^7.1 || ^8.0" + }, + "require-dev": { + "phpunit/phpunit": "^6.5 || ^8.5" + }, + "type": "library", + "autoload": { + "psr-4": { + "enshrined\\svgSanitize\\": "src" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "GPL-2.0-or-later" + ], + "authors": [ + { + "name": "Daryll Doyle", + "email": "daryll@enshrined.co.uk" + } + ], + "description": "An SVG sanitizer for PHP", + "support": { + "issues": "https://github.com/darylldoyle/svg-sanitizer/issues", + "source": "https://github.com/darylldoyle/svg-sanitizer/tree/0.21.0" + }, + "time": "2025-01-13T09:32:25+00:00" + }, { "name": "fortawesome/font-awesome", "version": "6.5.2", diff --git a/webapp/src/Controller/Jury/ProblemController.php b/webapp/src/Controller/Jury/ProblemController.php index d9256f2a6a..9253992658 100644 --- a/webapp/src/Controller/Jury/ProblemController.php +++ b/webapp/src/Controller/Jury/ProblemController.php @@ -605,16 +605,26 @@ public function testcasesAction(Request $request, int $probId): Response } $content = file_get_contents($file->getRealPath()); if ($type === 'image') { - $imageType = Utils::getImageType($content, $error); - if ($imageType === false) { - $this->addFlash('danger', sprintf('image: %s', $error)); - return $this->redirectToRoute('jury_problem_testcases', ['probId' => $probId]); - } - $thumb = Utils::getImageThumb($content, $thumbnailSize, - $this->dj->getDomjudgeTmpDir(), $error); - if ($thumb === false) { - $this->addFlash('danger', sprintf('image: %s', $error)); - return $this->redirectToRoute('jury_problem_testcases', ['probId' => $probId]); + if (mime_content_type($file->getRealPath()) === 'image/svg+xml') { + $content = Utils::sanitizeSvg($content); + if ($content === false) { + $this->addFlash('danger', sprintf('image: %s', $error)); + return $this->redirectToRoute('jury_problem_testcases', ['probId' => $probId]); + } + $thumb = $content; + $imageType = 'svg'; + } else { + $imageType = Utils::getImageType($content, $error); + if ($imageType === false) { + $this->addFlash('danger', sprintf('image: %s', $error)); + return $this->redirectToRoute('jury_problem_testcases', ['probId' => $probId]); + } + $thumb = Utils::getImageThumb($content, $thumbnailSize, + $this->dj->getDomjudgeTmpDir(), $error); + if ($thumb === false) { + $this->addFlash('danger', sprintf('image: %s', $error)); + return $this->redirectToRoute('jury_problem_testcases', ['probId' => $probId]); + } } $testcase->setImageType($imageType); diff --git a/webapp/src/Controller/Jury/SubmissionController.php b/webapp/src/Controller/Jury/SubmissionController.php index 90027eecc0..0927fdce0c 100644 --- a/webapp/src/Controller/Jury/SubmissionController.php +++ b/webapp/src/Controller/Jury/SubmissionController.php @@ -581,6 +581,7 @@ public function viewAction( 'requestedOutputCount' => $requestedOutputCount, 'version_warnings' => [], 'isMultiPassProblem' => $submission->getProblem()->isMultipassProblem(), + 'thumbnailSize' => $this->config->get('thumbnail_size'), ]; if ($selectedJudging === null) { diff --git a/webapp/src/Controller/Team/SubmissionController.php b/webapp/src/Controller/Team/SubmissionController.php index c26cffb046..5300b2273f 100644 --- a/webapp/src/Controller/Team/SubmissionController.php +++ b/webapp/src/Controller/Team/SubmissionController.php @@ -200,6 +200,7 @@ public function viewAction(Request $request, int $submitId): Response 'showSampleOutput' => $showSampleOutput, 'runs' => $runs, 'showTooLateResult' => $showTooLateResult, + 'thumbnailSize' => $this->config->get('thumbnail_size'), ]; if ($actuallyShowCompile) { $data['size'] = 'xl'; diff --git a/webapp/src/Service/DOMJudgeService.php b/webapp/src/Service/DOMJudgeService.php index 5e763f0985..7fc4b8115c 100644 --- a/webapp/src/Service/DOMJudgeService.php +++ b/webapp/src/Service/DOMJudgeService.php @@ -85,6 +85,12 @@ class DOMJudgeService 'image/svg+xml' => 'svg', ]; + final public const EXTENSION_TO_MIMETYPE = [ + 'png' => 'image/png', + 'jpg' => 'image/jpeg', + 'svg' => 'image/svg+xml', + ]; + public function __construct( protected readonly EntityManagerInterface $em, protected readonly LoggerInterface $logger, diff --git a/webapp/src/Service/ImportProblemService.php b/webapp/src/Service/ImportProblemService.php index f341187c93..9a705fc032 100644 --- a/webapp/src/Service/ImportProblemService.php +++ b/webapp/src/Service/ImportProblemService.php @@ -427,6 +427,15 @@ public function importZippedProblem( break; } } + // Handle SVG differently, as a lot of the above concepts do not make sense in this context. + $imageFileName = $baseFileName . '.svg'; + if (($imageFile = $zip->getFromName($imageFileName)) !== false) { + if (($imageFile = Utils::sanitizeSvg($imageFile)) === false) { + $messages['warning'][] = sprintf("Contents of '%s' is not safe.", $imageFileName); + } + $imageType = 'svg'; + $imageThumb = $imageFile; + } if (str_contains($testInput, "\r")) { $messages['warning'][] = "Testcase file '$baseFileName.in' contains Windows newlines."; diff --git a/webapp/src/Twig/TwigExtension.php b/webapp/src/Twig/TwigExtension.php index f66a0acff9..83b97c940d 100644 --- a/webapp/src/Twig/TwigExtension.php +++ b/webapp/src/Twig/TwigExtension.php @@ -117,6 +117,7 @@ public function getFilters(): array new TwigFilter('entityIdBadge', $this->entityIdBadge(...), ['is_safe' => ['html']]), new TwigFilter('medalType', $this->awards->medalType(...)), new TwigFilter('numTableActions', $this->numTableActions(...)), + new TwigFilter('extensionToMime', $this->extensionToMime(...)), ]; } @@ -1346,4 +1347,9 @@ protected function numTableActions(array $tableData): int } return $maxNumActions; } + + public function extensionToMime(string $extension): string + { + return DOMJudgeService::EXTENSION_TO_MIMETYPE[$extension]; + } } diff --git a/webapp/src/Utils/Utils.php b/webapp/src/Utils/Utils.php index b2a6c7bae6..419c635e77 100644 --- a/webapp/src/Utils/Utils.php +++ b/webapp/src/Utils/Utils.php @@ -3,6 +3,7 @@ use DateTime; use Doctrine\Inflector\InflectorFactory; +use enshrined\svgSanitize\Sanitizer as SvgSanitizer; use Symfony\Component\HttpFoundation\StreamedResponse; use Symfony\Component\HttpKernel\Exception\BadRequestHttpException; @@ -717,6 +718,15 @@ public static function getImageSize(string $filename): array return [$width, $height, $width / $height]; } + public static function sanitizeSvg(string $svgContents): string | false + { + $sanitizer = new SvgSanitizer(); + $sanitizer->removeRemoteReferences(true); + $sanitizer->minify(true); + + return $sanitizer->sanitize($svgContents); + } + /** * Returns TRUE iff string $haystack starts with string $needle. */ diff --git a/webapp/templates/jury/submission.html.twig b/webapp/templates/jury/submission.html.twig index 4b04e74b3a..9a17bc6ff9 100644 --- a/webapp/templates/jury/submission.html.twig +++ b/webapp/templates/jury/submission.html.twig @@ -19,6 +19,11 @@ .judging-table tr.disabled td a { color: silver } + + .image_thumb { + max-width: {{ thumbnailSize }}px; + max-height: {{ thumbnailSize }}px; + } {% endblock %} @@ -742,7 +747,7 @@ {% set imgUrl = path('jury_problem_testcase_fetch', {'probId': submission.problem.probid, 'rank': run.rank, 'type': 'image'}) %} - + {% endif %} diff --git a/webapp/tests/Unit/Utils/UtilsTest.php b/webapp/tests/Unit/Utils/UtilsTest.php index 15d60d8efb..f5ff542aa2 100644 --- a/webapp/tests/Unit/Utils/UtilsTest.php +++ b/webapp/tests/Unit/Utils/UtilsTest.php @@ -713,6 +713,55 @@ public function provideTestGetImageSize(): Generator yield [__DIR__ . '/../../../public/images/DOMjudgelogo.svg', 510, 1122]; } + public function testSanitizeSvg(): void + { + // SVG source: https://svg.enshrined.co.uk/ + $dirty = << + + + + + + + + + + + + + + + + + test 1 + test 2 + test 3 + test 4 + + test 5 + test 6 + + + + + + + + shouldn't be here + + + + + EOF; + $clean = Utils::sanitizeSvg($dirty); + self::assertFalse(str_contains($clean, "script")); + self::assertFalse(str_contains($clean, "alert")); + self::assertFalse(str_contains($clean, "shouldn't be here")); + self::assertFalse(str_contains($clean, "example.com")); + self::assertTrue(str_contains($clean, '')); + } + /** * Test that the wrapUnquoted function returns the correct result */