bug #7554 Improve the security of the File upload filed (javiereguiluz)

This PR was merged into the 4.x branch.

Discussion
----------

Improve the security of the File upload filed

Similar to #7551, #7552 and #7553.

This hardens `FileUploadType` against path-traversal risks flagged by the Claude security audit.

Commits
-------

cff061a Improve the security of the File upload filed

Author: javiereguiluz

src/Form/DataTransformer/StringToFileTransformer.php

@@ -75,13 +75,40 @@ private function doTransform(mixed $value): ?File
             throw new TransformationFailedException('Expected a string or null.');
         }
 
+        if (self::isUnsafeStoredPath($value)) {
+            return null;
+        }
+
         if (is_file($this->uploadDir.$value)) {
             return new File($this->uploadDir.$value);
         }
 
         return null;
     }
 
+    private static function isUnsafeStoredPath(string $value): bool
+    {
+        // reject empty values or null bytes
+        if ('' === $value || str_contains($value, "\0")) {
+            return true;
+        }
+
+        // reject absolute paths (Unix, UNC, Windows drive letter)
+        $normalized = str_replace('\\', '/', $value);
+        if (str_starts_with($normalized, '/') || 1 === preg_match('#^[a-zA-Z]:/#', $normalized)) {
+            return true;
+        }
+
+        // reject any `..` segment anywhere in the path
+        foreach (explode('/', $normalized) as $segment) {
+            if ('..' === $segment) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
     private function doReverseTransform(mixed $value): ?string
     {
         if (null === $value) {

src/Form/Type/FileUploadType.php

@@ -87,7 +87,16 @@ public function configureOptions(OptionsResolver $resolver): void
             unlink($file->getPathname());
         };
 
-        $uploadFilename = static fn (UploadedFile $file): string => $file->getClientOriginalName();
+        // the return value MUST be a safe relative path:
+        //   * no ".." segments
+        //   * no leading "/" or "\"
+        //   * no Windows drive letters
+        //   * no null bytes
+        //
+        // values that violate this contract are rejected on read (see
+        // StringToFileTransformer::doTransform) and the form behaves as if
+        // no file were stored. Overrides must preserve this contract.
+        $uploadFilename = static fn (UploadedFile $file): string => basename(str_replace('\\', '/', $file->getClientOriginalName()));
 
         $uploadValidate = static function (string $filename): string {
             if (!file_exists($filename)) {

tests/Unit/Form/DataTransformer/StringToFileTransformerTest.php

@@ -0,0 +1,86 @@
+<?php
+
+namespace EasyCorp\Bundle\EasyAdminBundle\Tests\Unit\Form\DataTransformer;
+
+use EasyCorp\Bundle\EasyAdminBundle\Form\DataTransformer\StringToFileTransformer;
+use PHPUnit\Framework\TestCase;
+use Symfony\Component\Filesystem\Filesystem;
+use Symfony\Component\HttpFoundation\File\File;
+
+class StringToFileTransformerTest extends TestCase
+{
+    private string $uploadDir;
+    private Filesystem $filesystem;
+
+    protected function setUp(): void
+    {
+        $this->filesystem = new Filesystem();
+        $this->uploadDir = sys_get_temp_dir().'/ea_stf_'.bin2hex(random_bytes(6)).'/';
+        $this->filesystem->mkdir($this->uploadDir.'subdir');
+        file_put_contents($this->uploadDir.'normal.txt', 'hello');
+        file_put_contents($this->uploadDir.'subdir/normal.txt', 'hello');
+    }
+
+    protected function tearDown(): void
+    {
+        $this->filesystem->remove($this->uploadDir);
+    }
+
+    public function testTransformReturnsNullForMissingFile(): void
+    {
+        $transformer = $this->createTransformer();
+
+        self::assertNull($transformer->transform('does-not-exist.txt'));
+    }
+
+    /**
+     * @dataProvider cleanRelativePathProvider
+     */
+    public function testTransformReturnsFileForCleanRelativePath(string $value): void
+    {
+        $transformer = $this->createTransformer();
+
+        $result = $transformer->transform($value);
+
+        self::assertInstanceOf(File::class, $result);
+    }
+
+    public static function cleanRelativePathProvider(): iterable
+    {
+        yield 'bare filename' => ['normal.txt'];
+        yield 'relative subpath' => ['subdir/normal.txt'];
+    }
+
+    /**
+     * @dataProvider unsafeStoredPathProvider
+     */
+    public function testTransformRejectsUnsafeStoredPath(string $value): void
+    {
+        $transformer = $this->createTransformer();
+
+        self::assertNull($transformer->transform($value));
+    }
+
+    public static function unsafeStoredPathProvider(): iterable
+    {
+        yield 'empty string' => [''];
+        yield 'parent traversal' => ['../../../etc/passwd'];
+        yield 'parent traversal inside subpath' => ['subdir/../../../etc/passwd'];
+        yield 'unix absolute path' => ['/etc/passwd'];
+        yield 'windows drive absolute' => ['C:/Windows/win.ini'];
+        yield 'windows backslash drive absolute' => ['C:\\Windows\\win.ini'];
+        yield 'backslash parent traversal' => ['sub\\..\\..\\etc'];
+        yield 'null byte' => ["file\0.txt"];
+        yield 'leading backslash' => ['\\etc\\passwd'];
+    }
+
+    private function createTransformer(): StringToFileTransformer
+    {
+        return new StringToFileTransformer(
+            $this->uploadDir,
+            static fn ($file) => 'noop',
+            static fn (string $filename) => $filename,
+            false,
+        );
+    }
+}