CVE-2023-28438: Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection') (CWE-89)
value: Pimcore is an open source data and experience management platform. Prior to version 10.5.19, since a user with 'report' permission can already write arbitrary SQL queries and given the fact that this endpoint is using the GET method (no CSRF protection), an attacker can inject an arbitrary query by manipulating a user to click on a link. Users should upgrade to version 10.5.19 to receive a patch or, as a workaround, may apply the patch manually.
CWE description: The product constructs all or part of an SQL command using externally-influenced input from an upstream component, but it does not neutralize or incorrectly neutralizes special elements that could modify the intended SQL command when it is sent to a downstream component. Without sufficient removal or quoting of SQL syntax in user-controllable inputs, the generated SQL query can cause those inputs to be interpreted as SQL instead of ordinary user data.
CVSS v3 base score: 8
Published: 2023-03-22T21:15Z
Repository: https://github.com/pimcore/pimcore
Language: PHP
Files changed: models/Tool/CustomReport/Adapter/Sql.php
Commit date: 2023-03-06 14:06:07 +0100
Commit message: [Task] Optimized custom reports filter (#14526)

* optimized custom reports filter

* Update models/Tool/CustomReport/Adapter/Sql.php

Co-authored-by: Jacob Dreesen <jacob@hdreesen.de>

---------

Co-authored-by: Jacob Dreesen <jacob@hdreesen.de>
Diff: diff --git a/models/Tool/CustomReport/Adapter/Sql.php b/models/Tool/CustomReport/Adapter/Sql.php
index e49426c59a06..ffb40108c7f0 100644
--- a/models/Tool/CustomReport/Adapter/Sql.php
+++ b/models/Tool/CustomReport/Adapter/Sql.php
@@ -36,11 +36,12 @@ class Sql extends AbstractAdapter
         if ($baseQuery) {
             $total = $db->fetchOne($baseQuery['count']);
 
             $order = '';
             if ($sort && $dir) {
-                $order = ' ORDER BY ' . $db->quoteIdentifier($sort) . ' ' . $dir;
+                $dir = ((strtoupper($dir) === 'ASC') ? 'ASC' : 'DESC');
+                $order = ' ORDER BY ' . $db->quoteIdentifier($sort) . ' ' .$dir;
             }
 
             $sql = $baseQuery['data'] . $order;
             if ($offset !== null && $limit) {
                 $sql .= " LIMIT $offset,$limit";
@@ -123,11 +124,11 @@ class Sql extends AbstractAdapter
         if ($drillDownFilters) {
             $havingParts = [];
             $db = Db::get();
             foreach ($drillDownFilters as $field => $value) {
                 if ($value !== '' && $value !== null) {
-                    $havingParts[] = "$field = " . $db->quote($value);
+                    $havingParts[] = ($db->quoteIdentifier($field) ." = " . $db->quote($value));
                 }
             }
 
             if ($havingParts) {
                 $sql .= ' HAVING ' . implode(' AND ', $havingParts);

Vulnerable code:                 $order = ' ORDER BY ' . $db->quoteIdentifier($sort) . ' ' . $dir;
                    $havingParts[] = "$field = " . $db->quote($value);
Fixed code:                 $dir = ((strtoupper($dir) === 'ASC') ? 'ASC' : 'DESC');
                $order = ' ORDER BY ' . $db->quoteIdentifier($sort) . ' ' .$dir;
                    $havingParts[] = ($db->quoteIdentifier($field) ." = " . $db->quote($value));