Whitelist PDF context options (#5924)
This commit is contained in:
24
composer.lock
generated
24
composer.lock
generated
@@ -1308,16 +1308,16 @@
|
|||||||
},
|
},
|
||||||
{
|
{
|
||||||
"name": "doctrine/migrations",
|
"name": "doctrine/migrations",
|
||||||
"version": "3.9.6",
|
"version": "3.9.7",
|
||||||
"source": {
|
"source": {
|
||||||
"type": "git",
|
"type": "git",
|
||||||
"url": "https://github.com/doctrine/migrations.git",
|
"url": "https://github.com/doctrine/migrations.git",
|
||||||
"reference": "ffd8355cdd8505fc650d9604f058bf62aedd80a1"
|
"reference": "96cb2a89b56c9efb0bac38e606dc0b0f13e650ec"
|
||||||
},
|
},
|
||||||
"dist": {
|
"dist": {
|
||||||
"type": "zip",
|
"type": "zip",
|
||||||
"url": "https://api.github.com/repos/doctrine/migrations/zipball/ffd8355cdd8505fc650d9604f058bf62aedd80a1",
|
"url": "https://api.github.com/repos/doctrine/migrations/zipball/96cb2a89b56c9efb0bac38e606dc0b0f13e650ec",
|
||||||
"reference": "ffd8355cdd8505fc650d9604f058bf62aedd80a1",
|
"reference": "96cb2a89b56c9efb0bac38e606dc0b0f13e650ec",
|
||||||
"shasum": ""
|
"shasum": ""
|
||||||
},
|
},
|
||||||
"require": {
|
"require": {
|
||||||
@@ -1391,7 +1391,7 @@
|
|||||||
],
|
],
|
||||||
"support": {
|
"support": {
|
||||||
"issues": "https://github.com/doctrine/migrations/issues",
|
"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": [
|
"funding": [
|
||||||
{
|
{
|
||||||
@@ -1407,7 +1407,7 @@
|
|||||||
"type": "tidelift"
|
"type": "tidelift"
|
||||||
}
|
}
|
||||||
],
|
],
|
||||||
"time": "2026-02-11T06:46:11+00:00"
|
"time": "2026-04-23T19:33:20+00:00"
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
"name": "doctrine/orm",
|
"name": "doctrine/orm",
|
||||||
@@ -2728,16 +2728,16 @@
|
|||||||
},
|
},
|
||||||
{
|
{
|
||||||
"name": "kevinpapst/tabler-bundle",
|
"name": "kevinpapst/tabler-bundle",
|
||||||
"version": "2.2.0",
|
"version": "2.2.1",
|
||||||
"source": {
|
"source": {
|
||||||
"type": "git",
|
"type": "git",
|
||||||
"url": "https://github.com/kevinpapst/TablerBundle.git",
|
"url": "https://github.com/kevinpapst/TablerBundle.git",
|
||||||
"reference": "d0f4130d7cba33015bbd798bfe529eeb4f19ac3f"
|
"reference": "236b9312e282437fe9df0c0a54004e80b497ec13"
|
||||||
},
|
},
|
||||||
"dist": {
|
"dist": {
|
||||||
"type": "zip",
|
"type": "zip",
|
||||||
"url": "https://api.github.com/repos/kevinpapst/TablerBundle/zipball/d0f4130d7cba33015bbd798bfe529eeb4f19ac3f",
|
"url": "https://api.github.com/repos/kevinpapst/TablerBundle/zipball/236b9312e282437fe9df0c0a54004e80b497ec13",
|
||||||
"reference": "d0f4130d7cba33015bbd798bfe529eeb4f19ac3f",
|
"reference": "236b9312e282437fe9df0c0a54004e80b497ec13",
|
||||||
"shasum": ""
|
"shasum": ""
|
||||||
},
|
},
|
||||||
"require": {
|
"require": {
|
||||||
@@ -2787,7 +2787,7 @@
|
|||||||
"description": "Admin/Backend theme bundle for Symfony based on Tabler.io",
|
"description": "Admin/Backend theme bundle for Symfony based on Tabler.io",
|
||||||
"support": {
|
"support": {
|
||||||
"issues": "https://github.com/kevinpapst/TablerBundle/issues",
|
"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": [
|
"funding": [
|
||||||
{
|
{
|
||||||
@@ -2799,7 +2799,7 @@
|
|||||||
"type": "github"
|
"type": "github"
|
||||||
}
|
}
|
||||||
],
|
],
|
||||||
"time": "2026-02-15T08:43:44+00:00"
|
"time": "2026-04-25T19:40:34+00:00"
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
"name": "league/csv",
|
"name": "league/csv",
|
||||||
|
|||||||
@@ -120,6 +120,19 @@ final class MPdfConverter implements HtmlToPdfConverter
|
|||||||
$mpdf->creator = Constants::SOFTWARE;
|
$mpdf->creator = Constants::SOFTWARE;
|
||||||
|
|
||||||
if (\count($associatedFiles) > 0) {
|
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);
|
$mpdf->SetAssociatedFiles($associatedFiles);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -15,10 +15,27 @@ namespace App\Pdf;
|
|||||||
*/
|
*/
|
||||||
final class PdfContext
|
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 = [];
|
private array $options = [];
|
||||||
|
|
||||||
public function setOption(string $key, string|int|array|null|bool $value): void
|
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;
|
$this->options[$key] = $value;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -37,4 +37,56 @@ class MPdfConverterTest extends KernelTestCase
|
|||||||
preg_match('/\/Creator \((.*)\)/', $result, $matches);
|
preg_match('/\/Creator \((.*)\)/', $result, $matches);
|
||||||
self::assertCount(2, $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('<h1>Test</h1>', [
|
||||||
|
'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);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -25,12 +25,40 @@ class PdfContextTest extends TestCase
|
|||||||
self::assertNull($sut->getOption('unknown'));
|
self::assertNull($sut->getOption('unknown'));
|
||||||
}
|
}
|
||||||
|
|
||||||
public function testSetterAndGetter(): void
|
public function testAllowedKeysRoundTrip(): void
|
||||||
{
|
{
|
||||||
$sut = new PdfContext();
|
$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', '<rdf:Description/>');
|
||||||
|
$sut->setOption('tempDir', '/tmp');
|
||||||
$sut->setOption('unknown', 'foo');
|
$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());
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1668,7 +1668,7 @@ parameters:
|
|||||||
|
|
||||||
-
|
-
|
||||||
message: "#^Parameter \\#2 \\$cacheDirectory of class App\\\\Pdf\\\\MPdfConverter constructor expects string, array\\|bool\\|float\\|int\\|string\\|null given\\.$#"
|
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
|
path: Pdf/MPdfConverterTest.php
|
||||||
|
|
||||||
-
|
-
|
||||||
|
|||||||
Reference in New Issue
Block a user