Match asset files on their extension instead of a substring

The custom CSS/JS and image lookups matched any filename containing
".css", ".js", ".png" and so on, so backup files like "theme.css.bak"
or a stray "data.json" were served as stylesheets or scripts. Every
directory was also checked against all extensions, so an image in
js/custom/ would have been included as a script.

Match on the filename suffix and let each caller pass the extensions
that are valid for its directory.

Author: meisterT

webapp/src/Service/DOMJudgeService.php

@@ -1425,31 +1425,34 @@ public function apiRelativeUrl(string $route, array $params = []): string
     }
 
     /**
-     * Get asset files in the given directory with the given extension
+     * Get asset files in the given directory ending in one of the given extensions
      *
+     * @param string[] $extensions
      * @return string[]
      */
-    public function getAssetFiles(string $path): array
+    public function getAssetFiles(string $path, array $extensions): array
     {
-        if (isset($this->assetFilesCache[$path])) {
-            return $this->assetFilesCache[$path];
+        $cacheKey = $path . '|' . implode(',', $extensions);
+        if (isset($this->assetFilesCache[$cacheKey])) {
+            return $this->assetFilesCache[$cacheKey];
         }
 
         $customDir = sprintf('%s/public/%s', $this->params->get('kernel.project_dir'), $path);
         if (!is_dir($customDir)) {
-            return $this->assetFilesCache[$path] = [];
+            return $this->assetFilesCache[$cacheKey] = [];
         }
 
         $results = [];
         foreach (scandir($customDir) as $file) {
-            foreach (array_merge(['css','js'], static::MIMETYPE_TO_EXTENSION) as $extension) {
-                if (str_contains($file, '.' . $extension)) {
+            foreach ($extensions as $extension) {
+                if (str_ends_with($file, '.' . $extension)) {
                     $results[] = $file;
+                    break;
                 }
             }
         }
 
-        return $this->assetFilesCache[$path] = $results;
+        return $this->assetFilesCache[$cacheKey] = $results;
     }
 
     /**
@@ -1475,7 +1478,7 @@ public function assetPath(?string $name, string $type, bool $fullPath = false, ?
         }
 
         if (isset($dir)) {
-            $assets = $this->getAssetFiles($dir);
+            $assets = $this->getAssetFiles($dir, array_values(static::MIMETYPE_TO_EXTENSION));
             foreach (static::MIMETYPE_TO_EXTENSION as $extension) {
                 if ($forceExtension === $extension || (!$forceExtension && in_array($name . '.' . $extension, $assets))) {
                     return sprintf('%s%s/%s.%s', $prefix, $dir, $name, $extension);

webapp/src/Twig/TwigExtension.php

@@ -988,7 +988,7 @@ public function printContestStart(Contest $contest): string
     public function customAssetFiles(string $type): array
     {
         if (in_array($type, ['css', 'js'])) {
-            return $this->dj->getAssetFiles("$type/custom");
+            return $this->dj->getAssetFiles("$type/custom", [$type]);
         }
 
         return [];

webapp/tests/Unit/Service/DOMJudgeServiceTest.php

@@ -0,0 +1,39 @@
+<?php declare(strict_types=1);
+
+namespace App\Tests\Unit\Service;
+
+use App\Service\DOMJudgeService;
+use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase;
+use Symfony\Component\Filesystem\Filesystem;
+
+class DOMJudgeServiceTest extends KernelTestCase
+{
+    private const ASSET_PATH = 'test-assets';
+
+    private DOMJudgeService $dj;
+    private string $assetDir;
+
+    protected function setUp(): void
+    {
+        self::bootKernel();
+        $this->dj       = static::getContainer()->get(DOMJudgeService::class);
+        $this->assetDir = static::getContainer()->getParameter('kernel.project_dir') . '/public/' . self::ASSET_PATH;
+        (new Filesystem())->mkdir($this->assetDir);
+    }
+
+    protected function tearDown(): void
+    {
+        (new Filesystem())->remove($this->assetDir);
+        parent::tearDown();
+    }
+
+    public function testGetAssetFilesMatchesOnTheExtensionOnly(): void
+    {
+        foreach (['ok.css', 'also-ok.css', 'backup.css.bak', 'data.json', 'script.js', 'image.png'] as $file) {
+            touch($this->assetDir . '/' . $file);
+        }
+
+        self::assertSame(['also-ok.css', 'ok.css'], $this->dj->getAssetFiles(self::ASSET_PATH, ['css']));
+        self::assertSame(['image.png', 'script.js'], $this->dj->getAssetFiles(self::ASSET_PATH, ['png', 'js']));
+    }
+}

webapp/tests/Unit/Twig/TwigExtensionTest.php

@@ -47,16 +47,18 @@ class TwigExtensionTest extends TestCase
     private const XSS_PAYLOAD = '<img src=x onerror="alert(1)">';
 
     private TwigExtension $twigExtension;
+    private DOMJudgeService&MockObject $dj;
     private RouterInterface&MockObject $router;
     private SerializerInterface&MockObject $serializer;
 
     protected function setUp(): void
     {
+        $this->dj         = $this->createMock(DOMJudgeService::class);
         $this->router     = $this->createMock(RouterInterface::class);
         $this->serializer = $this->createMock(SerializerInterface::class);
 
         $this->twigExtension = new TwigExtension(
-            $this->createMock(DOMJudgeService::class),
+            $this->dj,
             $this->createMock(ConfigurationService::class),
             $this->createMock(Environment::class),
             $this->createMock(EntityManagerInterface::class),
@@ -73,6 +75,16 @@ protected function setUp(): void
 
 
 
+    public function testCustomAssetFilesOnlyAsksForFilesOfItsOwnType(): void
+    {
+        $this->dj->expects(self::once())
+            ->method('getAssetFiles')
+            ->with('js/custom', ['js'])
+            ->willReturn(['a.js', 'b.js']);
+
+        self::assertSame(['a.js', 'b.js'], $this->twigExtension->customAssetFiles('js'));
+    }
+
     public function testPrintMetadataNull(): void
     {
         self::assertSame('', $this->twigExtension->printMetadata(null));