diff --git a/composer.lock b/composer.lock index c6dd3cef..eb3f7380 100644 --- a/composer.lock +++ b/composer.lock @@ -1308,16 +1308,16 @@ }, { "name": "doctrine/migrations", - "version": "3.9.6", + "version": "3.9.7", "source": { "type": "git", "url": "https://github.com/doctrine/migrations.git", - "reference": "ffd8355cdd8505fc650d9604f058bf62aedd80a1" + "reference": "96cb2a89b56c9efb0bac38e606dc0b0f13e650ec" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/doctrine/migrations/zipball/ffd8355cdd8505fc650d9604f058bf62aedd80a1", - "reference": "ffd8355cdd8505fc650d9604f058bf62aedd80a1", + "url": "https://api.github.com/repos/doctrine/migrations/zipball/96cb2a89b56c9efb0bac38e606dc0b0f13e650ec", + "reference": "96cb2a89b56c9efb0bac38e606dc0b0f13e650ec", "shasum": "" }, "require": { @@ -1391,7 +1391,7 @@ ], "support": { "issues": "https://github.com/doctrine/migrations/issues", - "source": "https://github.com/doctrine/migrations/tree/3.9.6" + "source": "https://github.com/doctrine/migrations/tree/3.9.7" }, "funding": [ { @@ -1407,7 +1407,7 @@ "type": "tidelift" } ], - "time": "2026-02-11T06:46:11+00:00" + "time": "2026-04-23T19:33:20+00:00" }, { "name": "doctrine/orm", @@ -2728,16 +2728,16 @@ }, { "name": "kevinpapst/tabler-bundle", - "version": "2.2.0", + "version": "2.2.1", "source": { "type": "git", "url": "https://github.com/kevinpapst/TablerBundle.git", - "reference": "d0f4130d7cba33015bbd798bfe529eeb4f19ac3f" + "reference": "236b9312e282437fe9df0c0a54004e80b497ec13" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/kevinpapst/TablerBundle/zipball/d0f4130d7cba33015bbd798bfe529eeb4f19ac3f", - "reference": "d0f4130d7cba33015bbd798bfe529eeb4f19ac3f", + "url": "https://api.github.com/repos/kevinpapst/TablerBundle/zipball/236b9312e282437fe9df0c0a54004e80b497ec13", + "reference": "236b9312e282437fe9df0c0a54004e80b497ec13", "shasum": "" }, "require": { @@ -2787,7 +2787,7 @@ "description": "Admin/Backend theme bundle for Symfony based on Tabler.io", "support": { "issues": "https://github.com/kevinpapst/TablerBundle/issues", - "source": "https://github.com/kevinpapst/TablerBundle/tree/2.2.0" + "source": "https://github.com/kevinpapst/TablerBundle/tree/2.2.1" }, "funding": [ { @@ -2799,7 +2799,7 @@ "type": "github" } ], - "time": "2026-02-15T08:43:44+00:00" + "time": "2026-04-25T19:40:34+00:00" }, { "name": "league/csv", diff --git a/src/Pdf/MPdfConverter.php b/src/Pdf/MPdfConverter.php index 9419bbec..b0ffdb95 100644 --- a/src/Pdf/MPdfConverter.php +++ b/src/Pdf/MPdfConverter.php @@ -120,6 +120,19 @@ final class MPdfConverter implements HtmlToPdfConverter $mpdf->creator = Constants::SOFTWARE; if (\count($associatedFiles) > 0) { + // remove "path" so mPDF will not use file_get_contents() on local files + // callers must pre-read and pass the bytes via "content" + $associatedFiles = array_map(static function ($entry): array { + if (!\is_array($entry)) { + return []; + } + + if (\array_key_exists('path', $entry)) { + unset($entry['path']); + } + + return $entry; + }, $associatedFiles); $mpdf->SetAssociatedFiles($associatedFiles); } diff --git a/src/Pdf/PdfContext.php b/src/Pdf/PdfContext.php index 4ae802b1..347dd704 100644 --- a/src/Pdf/PdfContext.php +++ b/src/Pdf/PdfContext.php @@ -15,10 +15,27 @@ namespace App\Pdf; */ final class PdfContext { + /** + * Keys that may be set from inside a (sandboxed) Twig template. + * + * Sinks that read the local filesystem inside mPDF (e.g. `associated_files` + * with a `path` entry, or `additional_xmp_rdf`) must NOT appear here. Those + * remain reachable via InvoiceModel::setOption() from PHP code. + */ + private const ALLOWED_KEYS = [ + 'filename', 'mode', 'format', 'orientation', 'default_font', 'default_font_size', 'fonts', + 'margin_left', 'margin_right', 'margin_top', 'margin_bottom', 'margin_header', 'margin_footer', + 'setAutoTopMargin', 'setAutoBottomMargin', 'PDFA', 'PDFAauto', 'useActiveForms', + ]; + private array $options = []; public function setOption(string $key, string|int|array|null|bool $value): void { + if (!\in_array($key, self::ALLOWED_KEYS, true)) { + return; + } + $this->options[$key] = $value; } diff --git a/tests/Pdf/MPdfConverterTest.php b/tests/Pdf/MPdfConverterTest.php index b5a55c78..383c7150 100644 --- a/tests/Pdf/MPdfConverterTest.php +++ b/tests/Pdf/MPdfConverterTest.php @@ -37,4 +37,56 @@ class MPdfConverterTest extends KernelTestCase preg_match('/\/Creator \((.*)\)/', $result, $matches); self::assertCount(2, $matches); } + + public function testAssociatedFilesPathIsStripped(): void + { + $kernel = self::bootKernel(); + $cacheDir = $kernel->getContainer()->getParameter('kernel.cache_dir'); + + // Plant a sentinel on disk that an attacker would try to exfiltrate via + // mPDF's `SetAssociatedFiles`. The legitimate ZUGFeRD path uses + // `content` (pre-read bytes); we additionally pass `path` to confirm + // it is stripped before reaching mPDF. + $sentinelPath = tempnam(sys_get_temp_dir(), 'kimai-pdf-leak-'); + self::assertNotFalse($sentinelPath); + $sentinelBytes = 'KIMAI_LEAK_SENTINEL_' . bin2hex(random_bytes(8)); + file_put_contents($sentinelPath, $sentinelBytes); + + $legitimateContent = 'KIMAI_LEGITIMATE_CONTENT_' . bin2hex(random_bytes(8)); + + try { + $sut = new MPdfConverter((new FileHelperFactory($this))->create(), $cacheDir); + $result = $sut->convertToPdf('

Test

', [ + 'associated_files' => [ + [ + 'name' => 'attachment.txt', + 'mime' => 'text/plain', + 'description' => 'mixed entry', + 'AFRelationship' => 'Alternative', + 'path' => $sentinelPath, + 'content' => $legitimateContent, + ], + ], + ]); + } finally { + @unlink($sentinelPath); + } + + self::assertNotEmpty($result); + + // Decompress every FlateDecode stream in the produced PDF. The + // sentinel must not appear; the explicitly-supplied `content` must. + $allDecompressed = ''; + if (preg_match_all('/stream\r?\n(.*?)\r?\nendstream/s', $result, $streams) > 0) { + foreach ($streams[1] as $stream) { + $decoded = @gzuncompress($stream); + if ($decoded !== false) { + $allDecompressed .= $decoded; + } + } + } + self::assertStringNotContainsString($sentinelBytes, $result); + self::assertStringNotContainsString($sentinelBytes, $allDecompressed); + self::assertStringContainsString($legitimateContent, $allDecompressed); + } } diff --git a/tests/Pdf/PdfContextTest.php b/tests/Pdf/PdfContextTest.php index c690d29b..21b0d6a6 100644 --- a/tests/Pdf/PdfContextTest.php +++ b/tests/Pdf/PdfContextTest.php @@ -25,12 +25,40 @@ class PdfContextTest extends TestCase self::assertNull($sut->getOption('unknown')); } - public function testSetterAndGetter(): void + public function testAllowedKeysRoundTrip(): void { $sut = new PdfContext(); - self::assertNull($sut->getOption('unknown')); + $sut->setOption('margin_top', '12'); + $sut->setOption('format', 'A4-P'); + $sut->setOption('PDFA', true); + $sut->setOption('fonts', ['custom' => ['R' => 'custom.ttf']]); + + self::assertEquals('12', $sut->getOption('margin_top')); + self::assertEquals('A4-P', $sut->getOption('format')); + self::assertTrue($sut->getOption('PDFA')); + self::assertEquals(['custom' => ['R' => 'custom.ttf']], $sut->getOption('fonts')); + self::assertCount(4, $sut->getOptions()); + } + + /** + * Templates running in the Twig sandbox must not be able to push the + * file-disclosure sinks (`associated_files` with `path`, `additional_xmp_rdf`) + * or arbitrary mPDF config keys through PdfContext. + */ + public function testForbiddenAndUnknownKeysAreDropped(): void + { + $sut = new PdfContext(); + + $sut->setOption('associated_files', [['path' => '/etc/passwd']]); + $sut->setOption('additional_xmp_rdf', ''); + $sut->setOption('tempDir', '/tmp'); $sut->setOption('unknown', 'foo'); - self::assertEquals('foo', $sut->getOption('unknown')); + + self::assertNull($sut->getOption('associated_files')); + self::assertNull($sut->getOption('additional_xmp_rdf')); + self::assertNull($sut->getOption('tempDir')); + self::assertNull($sut->getOption('unknown')); + self::assertEmpty($sut->getOptions()); } } diff --git a/tests/phpstan.neon b/tests/phpstan.neon index 749a0009..4b79ca9e 100644 --- a/tests/phpstan.neon +++ b/tests/phpstan.neon @@ -1668,7 +1668,7 @@ parameters: - message: "#^Parameter \\#2 \\$cacheDirectory of class App\\\\Pdf\\\\MPdfConverter constructor expects string, array\\|bool\\|float\\|int\\|string\\|null given\\.$#" - count: 1 + count: 2 path: Pdf/MPdfConverterTest.php -