From dce6577e8b55e1df30dca289552312c64f2a633f Mon Sep 17 00:00:00 2001 From: weisswurstkanone Date: Tue, 10 Aug 2021 15:47:01 +0200 Subject: [PATCH 1/6] CSV export --- models/DataObject/Service.php | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/models/DataObject/Service.php b/models/DataObject/Service.php index 33ed4fb067d..b2e6058b189 100644 --- a/models/DataObject/Service.php +++ b/models/DataObject/Service.php @@ -1798,6 +1798,7 @@ public static function getCsvDataForObject(AbstractObject $object, $requestedLan if (!isset($mappedFieldnames[$localizedFieldKey])) { $mappedFieldnames[$localizedFieldKey] = $mappedFieldnameBase . '-' . $validLanguage; } + self::sanitizeCsvFieldData($fieldData); $objectData[$localizedFieldKey] = $fieldData; } @@ -1807,8 +1808,11 @@ public static function getCsvDataForObject(AbstractObject $object, $requestedLan if (!isset($mappedFieldnames[$field])) { $mappedFieldnames[$field] = self::mapFieldname($field, $helperDefinitions); } + self::sanitizeCsvFieldData($fieldData); $objectData[$field] = $fieldData; } + + } if ($returnMappedFieldNames) { @@ -1873,6 +1877,18 @@ public static function getCsvData($requestedLanguage, LocaleServiceInterface $lo return $data; } + /** + * @internal + * @param $data + */ + public static function sanitizeCsvFieldData(&$data) { + if (preg_match('/^[=\+\-@]/', $data)) { + // prevent formula injection + // see https://www.contextis.com/en/blog/comma-separated-vulnerabilities + $data = "'" . $data; + } + } + /** * @param string $field * @param array $helperDefinitions From ad03d978d3be161fabb9b95cbb9832b9d99e44cd Mon Sep 17 00:00:00 2001 From: weisswurstkanone Date: Tue, 10 Aug 2021 15:52:49 +0200 Subject: [PATCH 2/6] CSV export --- models/DataObject/Service.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/models/DataObject/Service.php b/models/DataObject/Service.php index b2e6058b189..fe5d2a67223 100644 --- a/models/DataObject/Service.php +++ b/models/DataObject/Service.php @@ -1879,9 +1879,9 @@ public static function getCsvData($requestedLanguage, LocaleServiceInterface $lo /** * @internal - * @param $data + * @param ?string $data */ - public static function sanitizeCsvFieldData(&$data) { + public static function sanitizeCsvFieldData(?string &$data) { if (preg_match('/^[=\+\-@]/', $data)) { // prevent formula injection // see https://www.contextis.com/en/blog/comma-separated-vulnerabilities From d247a178c9766da8d7c9f10fb223a5c82d76997c Mon Sep 17 00:00:00 2001 From: weisswurstkanone Date: Tue, 17 Aug 2021 15:22:01 +0200 Subject: [PATCH 3/6] [Data Object] CSV export: prevent formula injection #9992 --- .../Admin/Asset/AssetHelperController.php | 1 + .../Admin/TranslationController.php | 4 +++- .../Reports/CustomReportController.php | 2 ++ lib/Routing/Redirect/Csv.php | 2 ++ models/DataObject/Service.php | 18 ++++----------- models/Element/Service.php | 23 +++++++++++++++++++ 6 files changed, 35 insertions(+), 15 deletions(-) diff --git a/bundles/AdminBundle/Controller/Admin/Asset/AssetHelperController.php b/bundles/AdminBundle/Controller/Admin/Asset/AssetHelperController.php index 95f78202f5c..ea10f635b56 100644 --- a/bundles/AdminBundle/Controller/Admin/Asset/AssetHelperController.php +++ b/bundles/AdminBundle/Controller/Admin/Asset/AssetHelperController.php @@ -763,6 +763,7 @@ protected function getCsvData(Request $request, $language, $list, $fields, $addT } $dataRows[] = $data; } + $dataRows = Element\Service::escapeCsvRecord($dataRows); $csv[] = $dataRows; } } diff --git a/bundles/AdminBundle/Controller/Admin/TranslationController.php b/bundles/AdminBundle/Controller/Admin/TranslationController.php index 2755b083f28..102c873177c 100644 --- a/bundles/AdminBundle/Controller/Admin/TranslationController.php +++ b/bundles/AdminBundle/Controller/Admin/TranslationController.php @@ -219,12 +219,14 @@ public function exportAction(Request $request) } foreach ($translationObjects as $t) { + $row = $t->getTranslations(); + $row = Element\Service::escapeCsvRecord($row); $translations[] = array_merge( ['key' => $t->getKey(), 'creationDate' => $t->getCreationDate(), 'modificationDate' => $t->getModificationDate(), ], - $t->getTranslations() + $row ); } diff --git a/bundles/AdminBundle/Controller/Reports/CustomReportController.php b/bundles/AdminBundle/Controller/Reports/CustomReportController.php index 737543e8528..54cffa02e44 100644 --- a/bundles/AdminBundle/Controller/Reports/CustomReportController.php +++ b/bundles/AdminBundle/Controller/Reports/CustomReportController.php @@ -15,6 +15,7 @@ namespace Pimcore\Bundle\AdminBundle\Controller\Reports; +use Pimcore\Model\Element\Service; use Pimcore\Model\Tool\CustomReport; use Symfony\Component\Filesystem\Exception\FileNotFoundException; use Symfony\Component\HttpFoundation\BinaryFileResponse; @@ -423,6 +424,7 @@ public function createCsvAction(Request $request) } foreach ($result['data'] as $row) { + $row = Service::escapeCsvRecord($row); fputcsv($fp, array_values($row), ';'); } diff --git a/lib/Routing/Redirect/Csv.php b/lib/Routing/Redirect/Csv.php index 0637e4c4520..16e8be7cb5f 100644 --- a/lib/Routing/Redirect/Csv.php +++ b/lib/Routing/Redirect/Csv.php @@ -22,6 +22,7 @@ use League\Csv\Statement; use League\Csv\Writer; use Pimcore\Model\Document; +use Pimcore\Model\Element\Service; use Pimcore\Model\Redirect; use Pimcore\Tool\Admin; use Pimcore\Tool\ArrayNormalizer; @@ -110,6 +111,7 @@ public function createExportWriter(Redirect\Listing $list): Writer $redirect->getActive(), $expiry, ]; + $data = Service::escapeCsvRecord($data); $writer->insertOne($data); } diff --git a/models/DataObject/Service.php b/models/DataObject/Service.php index fe5d2a67223..3fc96c43c6e 100644 --- a/models/DataObject/Service.php +++ b/models/DataObject/Service.php @@ -1808,7 +1808,7 @@ public static function getCsvDataForObject(AbstractObject $object, $requestedLan if (!isset($mappedFieldnames[$field])) { $mappedFieldnames[$field] = self::mapFieldname($field, $helperDefinitions); } - self::sanitizeCsvFieldData($fieldData); + $objectData[$field] = $fieldData; } @@ -1870,25 +1870,15 @@ public static function getCsvData($requestedLanguage, LocaleServiceInterface $lo $data[] = $tmp; } - $data[] = self::getCsvDataForObject($object, $requestedLanguage, $fields, $helperDefinitions, $localeService, $context); + $rowData = self::getCsvDataForObject($object, $requestedLanguage, $fields, $helperDefinitions, $localeService, $context);; + $rowData = self::escapeCsvRecord($rowData); + $data[] = $rowData; } } return $data; } - /** - * @internal - * @param ?string $data - */ - public static function sanitizeCsvFieldData(?string &$data) { - if (preg_match('/^[=\+\-@]/', $data)) { - // prevent formula injection - // see https://www.contextis.com/en/blog/comma-separated-vulnerabilities - $data = "'" . $data; - } - } - /** * @param string $field * @param array $helperDefinitions diff --git a/models/Element/Service.php b/models/Element/Service.php index ea7c06eb6f7..ac625e7910a 100644 --- a/models/Element/Service.php +++ b/models/Element/Service.php @@ -22,6 +22,8 @@ use DeepCopy\Matcher\PropertyTypeMatcher; use Doctrine\Common\Collections\Collection; use Doctrine\DBAL\Query\QueryBuilder as DoctrineQueryBuilder; +use League\Csv\EscapeFormula; +use League\Csv\Writer; use Pimcore\Db; use Pimcore\Event\SystemEvents; use Pimcore\File; @@ -49,6 +51,12 @@ */ class Service extends Model\AbstractModel { + + /** + * @var EscapeFormula + */ + protected static $formatter = null; + /** * @internal * @@ -1539,4 +1547,19 @@ function ($currentValue) { return $event->getArgument('copier'); } + + /** + * @internal + * + * @param array $rowData + * @return array + */ + public static function escapeCsvRecord(array $rowData): array { + if (!self::$formatter) { + self::$formatter = new EscapeFormula("'", ['=', '-' , '+', '@']); + } + $rowData = self::$formatter->escapeRecord($rowData); + return $rowData; + } + } From 90ebf1f20b0c0b32f9dfb3539ba9cb5ea6dd82ce Mon Sep 17 00:00:00 2001 From: weisswurstkanone Date: Tue, 17 Aug 2021 15:28:16 +0200 Subject: [PATCH 4/6] [Data Object] CSV export: prevent formula injection #9992 --- models/DataObject/Service.php | 1 - models/Element/Service.php | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/models/DataObject/Service.php b/models/DataObject/Service.php index 3fc96c43c6e..922bb898f79 100644 --- a/models/DataObject/Service.php +++ b/models/DataObject/Service.php @@ -1798,7 +1798,6 @@ public static function getCsvDataForObject(AbstractObject $object, $requestedLan if (!isset($mappedFieldnames[$localizedFieldKey])) { $mappedFieldnames[$localizedFieldKey] = $mappedFieldnameBase . '-' . $validLanguage; } - self::sanitizeCsvFieldData($fieldData); $objectData[$localizedFieldKey] = $fieldData; } diff --git a/models/Element/Service.php b/models/Element/Service.php index ac625e7910a..d0691dd9dd0 100644 --- a/models/Element/Service.php +++ b/models/Element/Service.php @@ -1555,7 +1555,7 @@ function ($currentValue) { * @return array */ public static function escapeCsvRecord(array $rowData): array { - if (!self::$formatter) { + if (self::$formatter === null) { self::$formatter = new EscapeFormula("'", ['=', '-' , '+', '@']); } $rowData = self::$formatter->escapeRecord($rowData); From dc3c02810353cbca5d420bb7211aedcaf7132f00 Mon Sep 17 00:00:00 2001 From: Josef Aichhorn Date: Tue, 17 Aug 2021 16:03:14 +0200 Subject: [PATCH 5/6] Update models/Element/Service.php Co-authored-by: Bernhard Rusch --- models/Element/Service.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/Element/Service.php b/models/Element/Service.php index d0691dd9dd0..e8fa8a9736d 100644 --- a/models/Element/Service.php +++ b/models/Element/Service.php @@ -55,7 +55,7 @@ class Service extends Model\AbstractModel /** * @var EscapeFormula */ - protected static $formatter = null; + private static ?EscapeFormula $formatter = null; /** * @internal From bc966df15ae26699a3a465d6ab5d8715ab5a2894 Mon Sep 17 00:00:00 2001 From: Bernhard Rusch Date: Tue, 17 Aug 2021 17:31:12 +0200 Subject: [PATCH 6/6] Update models/Element/Service.php --- models/Element/Service.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/Element/Service.php b/models/Element/Service.php index e8fa8a9736d..0cd23c210fb 100644 --- a/models/Element/Service.php +++ b/models/Element/Service.php @@ -53,7 +53,7 @@ class Service extends Model\AbstractModel { /** - * @var EscapeFormula + * @var EscapeFormula|null */ private static ?EscapeFormula $formatter = null;