From c6c80905e58c7724c776f980570a56df7016c6d1 Mon Sep 17 00:00:00 2001 From: Matthias Schuhmayer <38959016+mattamon@users.noreply.github.com> Date: Thu, 20 Apr 2023 10:07:32 +0200 Subject: [PATCH] [Security] Fix Admin Translation Export SQL Injection (#14968) * Fix admin translation export sql injection * Change to setConditon * Add SyntaxError Exception, replace sql comments * Remove empty line * Use const, remove unecessary line --- .../Admin/TranslationController.php | 70 ++++++++++++++----- 1 file changed, 51 insertions(+), 19 deletions(-) diff --git a/bundles/AdminBundle/Controller/Admin/TranslationController.php b/bundles/AdminBundle/Controller/Admin/TranslationController.php index 2e78979caf0..16a5a132ca2 100644 --- a/bundles/AdminBundle/Controller/Admin/TranslationController.php +++ b/bundles/AdminBundle/Controller/Admin/TranslationController.php @@ -15,6 +15,7 @@ namespace Pimcore\Bundle\AdminBundle\Controller\Admin; +use Doctrine\DBAL\Exception\SyntaxErrorException; use Doctrine\DBAL\Query\QueryBuilder as DoctrineQueryBuilder; use Pimcore\Bundle\AdminBundle\Controller\AdminController; use Pimcore\File; @@ -49,6 +50,8 @@ */ class TranslationController extends AdminController { + protected const PLACEHOLDER_NAME = 'placeHolder'; + /** * @Route("/import", name="pimcore_admin_translation_import", methods={"POST"}) * @@ -183,9 +186,9 @@ public function exportAction(Request $request) $list->setOrder('asc'); $list->setOrderKey($tableName . '.key', false); - $condition = $this->getGridFilterCondition($request, $tableName, false, $admin); - if ($condition) { - $list->setCondition($condition); + $conditions = $this->getGridFilterCondition($request, $tableName, false, $admin); + if (!empty($conditions)) { + $list->setCondition($conditions['condition'], $conditions['params']); } $filters = $this->getGridFilterCondition($request, $tableName, true, $admin); @@ -195,7 +198,12 @@ public function exportAction(Request $request) } $this->extendTranslationQuery($joins, $list, $tableName, $filters); - $list->load(); + + try { + $list->load(); + } catch (SyntaxErrorException $syntaxErrorException) { + throw new \InvalidArgumentException('Check your arguments.'); + } $translations = []; $translationObjects = $list->getTranslations(); @@ -460,19 +468,24 @@ public function translationsAction(Request $request, TranslatorInterface $transl $list->setLimit($request->get('limit')); $list->setOffset($request->get('start')); - $condition = $this->getGridFilterCondition($request, $tableName, false, $admin); + $conditions = $this->getGridFilterCondition($request, $tableName, false, $admin); $filters = $this->getGridFilterCondition($request, $tableName, true, $admin); if ($filters) { $joins = array_merge($joins, $filters['joins']); } - if ($condition) { - $list->setCondition($condition); + + if (!empty($conditions)) { + $list->setCondition($conditions['condition'], $conditions['params']); } $this->extendTranslationQuery($joins, $list, $tableName, $filters); - $list->load(); + try { + $list->load(); + } catch (SyntaxErrorException $syntaxErrorException) { + throw new \InvalidArgumentException('Check your arguments.'); + } $translations = []; $searchString = $request->get('searchString'); @@ -574,11 +587,12 @@ function (DoctrineQueryBuilder $select) use ( * @param Request $request * @param string $tableName * @param bool $languageMode - * - * @return array|null|string + * @param bool $admin + * @return array */ protected function getGridFilterCondition(Request $request, $tableName, $languageMode = false, $admin = false) { + $placeHolderCount = 0; $joins = []; $conditions = []; $validLanguages = $admin ? Tool\Admin::getLanguages() : $this->getAdminUser()->getAllowedLanguagesForViewingWebsiteTranslations(); @@ -592,6 +606,7 @@ protected function getGridFilterCondition(Request $request, $tableName, $languag $operatorField = 'operator'; $filters = $this->decodeJson($filterJson); + foreach ($filters as $filter) { $operator = '='; $field = null; @@ -601,6 +616,7 @@ protected function getGridFilterCondition(Request $request, $tableName, $languag if (in_array(ltrim($fieldname, '_'), $validLanguages)) { $fieldname = ltrim($fieldname, '_'); } + $fieldname = str_replace('--', '', $fieldname); if (!$languageMode && in_array($fieldname, $validLanguages) || $languageMode && !in_array($fieldname, $validLanguages)) { @@ -641,31 +657,47 @@ protected function getGridFilterCondition(Request $request, $tableName, $languag 'language' => $fieldname, ]; } else { - $conditionFilters[] = $condition; + $placeHolderName = self::PLACEHOLDER_NAME . $placeHolderCount; + $placeHolderCount++; + $conditionFilters[] = [ + 'condition' => $field . ' ' . $operator . ' :' . $placeHolderName, + 'field' => $placeHolderName, + 'value' => $value + ]; } } } } if ($request->get('searchString')) { - $filterTerm = $db->quote('%' . mb_strtolower($request->get('searchString')) . '%'); - $conditionFilters[] = '(lower(' . $tableName . '.key) LIKE ' . $filterTerm . ' OR lower(' . $tableName . '.text) LIKE ' . $filterTerm . ')'; + $conditionFilters[] = [ + 'condition' => '(lower(' . $tableName . '.key) LIKE :filterTerm OR lower(' . $tableName . '.text) LIKE :filterTerm)', + 'field' => 'filterTerm', + 'value' => '%' . mb_strtolower($request->get('searchString')) . '%' + ]; } if ($languageMode) { - $result = [ + return [ 'joins' => $joins, 'conditions' => $conditions, ]; + } - return $result; - } else { - if (!empty($conditionFilters)) { - return implode(' AND ', $conditionFilters); + if(!empty($conditionFilters)) { + $conditions = []; + $params = []; + foreach($conditionFilters as $conditionFilter) { + $conditions[] = $conditionFilter['condition']; + $params[$conditionFilter['field']] = $conditionFilter['value']; } - return null; + $conditionFilters = [ + 'condition' => implode(' AND ', $conditions), + 'params' => $params + ]; } + return $conditionFilters; } /**