[TikiWiki-commits] [Git][tikiwiki/tiki][master] [FIX] Sanitize ORDER BY direction in performance stats to prevent SQL injection

Alfred Syatsukwa pushed to branch master at Tiki Wiki CMS Groupware / Tiki


Commits:
94fd808f by Alfred Syatsukwa at 2026-08-18T16:58:28+00:00
[FIX] Sanitize ORDER BY direction in performance stats to prevent SQL injection
---
* [FIX] Sanitize ORDER BY direction in performance stats to prevent SQL injection

(cherry picked from commit 2dc0f61a2f84c472269fea388bd14827876871ee)

See merge request tikiwiki/tiki!10955

- - - - -


8 changed files:

- lib/Filegals/FileGalLib.php
- lib/Logs/LogsQueryLib.php
- lib/core/Search/Manticore/FacetBuilder.php
- lib/core/Search/Manticore/PdoClient.php
- lib/core/Tracker/Query.php
- lib/freetag/freetaglib.php
- lib/performance/performancestatslib.php
- tiki-performance_stats.php


Changes:

=====================================
lib/Filegals/FileGalLib.php
=====================================
@@ -3119,10 +3119,9 @@ class FileGalLib extends TikiLib
             $numQuery = preg_replace("/ ORDER BY .*$/", "", $query);
             $numQuery = "SELECT COUNT(*) FROM (" . $numQuery . ") AS grouped";
             $numResults = $this->getOne($numQuery, $bindvars);
-            $limit = $offset == -1 ? 0 : $offset;
-            $limit .= ', ' . ($maxRecords == -1 ? PHP_INT_MAX : $maxRecords);
-            $query .= " LIMIT $limit";
-            $result = $this->fetchAll($query, $bindvars);
+            $fetchOffset = ($offset == -1) ? -1 : (int)$offset;
+            $fetchMaxRecords = ($maxRecords == -1) ? -1 : (int)$maxRecords;
+            $result = $this->fetchAll($query, $bindvars, $fetchMaxRecords, $fetchOffset);
         } else {
             $result = $this->fetchAll($query, $bindvars);
             $numResults = count($result);


=====================================
lib/Logs/LogsQueryLib.php
=====================================
@@ -287,13 +287,10 @@ class LogsQueryLib
             " . ($this->groupType == "countByDate" ? " GROUP BY DATE_FORMAT(FROM_UNIXTIME(lastModif), '%Y%m%d') " : "") . "
 
             ORDER BY lastModif " . ($this->desc == true ? "DESC" : "ASC") . "
-
-            " . (! empty($this->limit) ?
-                " LIMIT " . $this->limit
-                : ""
-            ) . "
         ";
 
+        $numrows = ! empty($this->limit) ? (int)$this->limit : -1;
+
         $params = [$this->type];
 
         if (! empty($this->id)) {
@@ -315,7 +312,7 @@ class LogsQueryLib
         if ($this->groupType == "count") {
             return $tikilib->getOne($query, $params);
         } else {
-            return $tikilib->fetchAll($query, $params);
+            return $tikilib->fetchAll($query, $params, $numrows);
         }
     }
 }


=====================================
lib/core/Search/Manticore/FacetBuilder.php
=====================================
@@ -81,7 +81,8 @@ class FacetBuilder
             } else {
                 $out .= ' ORDER BY COUNT(*) DESC';
             }
-            $out .= ' LIMIT ' . $count;
+            // Cast to int to prevent SQL injection. SphinxQL FACET syntax does not support bound parameters
+            $out .= ' LIMIT ' . (int)$count;
         }
 
         return $out;


=====================================
lib/core/Search/Manticore/PdoClient.php
=====================================
@@ -453,6 +453,9 @@ class PdoClient
         } else {
             $sql .= " ORDER BY weight() desc, id asc";
         }
+        // Cast to int to prevent SQL injection — SphinxQL LIMIT does not support bound parameters
+        $resultStart = (int)$resultStart;
+        $resultCount = (int)$resultCount;
         $sql .= " LIMIT $resultStart, $resultCount option not_terms_only_allowed=1,cutoff=0,expand_keywords=1";
         if ($resultStart + $resultCount > 1000) {
             $sql .= ',max_matches=' . ($resultStart + $resultCount);


=====================================
lib/core/Tracker/Query.php
=====================================
@@ -987,17 +987,18 @@ class Tracker_Query
                 " . ($isSearch == true ? ", search_item_fields.fieldId, search_item_fields.itemId " : "") . "
                 ORDER BY
                 tiki_tracker_items." . $dateUnit . " " . ($this->desc == true ? 'DESC' : 'ASC') . "
-                " . (! empty($this->limit) ? " LIMIT " . $this->limit : "") . "
-                " . (! empty($this->offset) ? " OFFSET " . $this->offset : "");
+                ";
+
+        $numrows = ! empty($this->limit) ? (int)$this->limit : -1;
+        $fetchOffset = ! empty($this->offset) ? (int)$this->offset : -1;
 
         if ($this->debug == true) {
-            $result = [$query, $params];
-            // @phpstan-ignore disallowedFunctions.printR (intentional debug output gated by $this->debug)
-            print_r($result);
-            print_r($tikilib->fetchAll($query, $params)); // @phpstan-ignore disallowedFunctions.printR (intentional debug output gated by $this->debug)
+            $debugOutput = var_export([$query, $params], true) . "\n";
+            $debugOutput .= var_export($tikilib->fetchAll($query, $params, $numrows, $fetchOffset), true);
+            echo $debugOutput;
             die;
         } else {
-            $result = $tikilib->fetchAll($query, $params);
+            $result = $tikilib->fetchAll($query, $params, $numrows, $fetchOffset);
         }
 
         $newResult = [];


=====================================
lib/freetag/freetaglib.php
=====================================
@@ -1157,13 +1157,18 @@ class FreetagLib extends ObjectLib
         if (! $lang) {
             $lang = Language::getCurrentLanguage();
         }
+
+        $max = filter_var($max, FILTER_VALIDATE_INT, ['options' => ['min_range' => 1]]);
+        if ($max === false) {
+            $max = 10;
+        }
+
         $query = 'SELECT t.* FROM `tiki_freetags` t, `tiki_freetagged_objects` o'
                         . ' WHERE t.`tagId` = o.`tagId`'
                         . ' AND (`lang` = ? or `lang` IS null)'
-                        . ' ORDER BY ' . $this->convertSortMode('random')
-                        . ' LIMIT ' . $max;
+                        . ' ORDER BY ' . $this->convertSortMode('random');
 
-        $result = $this->query($query, [ $lang ]);
+        $result = $this->query($query, [ $lang ], $max);
 
         $tags = [];
         $index = [];


=====================================
lib/performance/performancestatslib.php
=====================================
@@ -52,11 +52,14 @@ class PerformanceStatsLib extends TikiLib
      */
     public function getRequestsBasedOnAverageRequestTime(int $amount = 25, int $offset = 0, string $find = '', string $order = 'DESC', string $orderType = '')
     {
-        $order = strtoupper($order) === 'ASC' ? 'ASC' : 'DESC';
+        $order = strtoupper($order);
+        if (! in_array($order, ['ASC', 'DESC'])) {
+            $order = 'DESC';
+        }
         if ($orderType == 'no_of_requests') {
-            return $this->query("SELECT url, round(AVG(time_taken)) AS average_time_taken, COUNT(url) AS number_of_requests FROM tiki_performance WHERE url LIKE ? GROUP BY url ORDER BY number_of_requests $order LIMIT $amount OFFSET $offset", ["%$find%"]);
+            return $this->query("SELECT url, round(AVG(time_taken)) AS average_time_taken, COUNT(url) AS number_of_requests FROM tiki_performance WHERE url LIKE ? GROUP BY url ORDER BY number_of_requests $order", ["%$find%"], $amount, $offset);
         } else {
-            return $this->query("SELECT url, round(AVG(time_taken)) AS average_time_taken, COUNT(url) AS number_of_requests FROM tiki_performance WHERE url LIKE ? GROUP BY url ORDER BY average_time_taken $order LIMIT $amount OFFSET $offset", ["%$find%"]);
+            return $this->query("SELECT url, round(AVG(time_taken)) AS average_time_taken, COUNT(url) AS number_of_requests FROM tiki_performance WHERE url LIKE ? GROUP BY url ORDER BY average_time_taken $order", ["%$find%"], $amount, $offset);
         }
     }
 
@@ -70,8 +73,11 @@ class PerformanceStatsLib extends TikiLib
      */
     public function getRequestsBasedOnMaximumProcessingTime(int $amount = 25, int $offset = 0, string $find = '', string $order = 'DESC')
     {
-        $order = strtoupper($order) === 'ASC' ? 'ASC' : 'DESC';
-        return $this->query("SELECT url, MAX(time_taken) AS maximum_time_taken FROM tiki_performance WHERE url LIKE ? GROUP BY url ORDER BY maximum_time_taken $order LIMIT $amount OFFSET $offset", ["%$find%"]);
+        $order = strtoupper($order);
+        if (! in_array($order, ['ASC', 'DESC'])) {
+            $order = 'DESC';
+        }
+        return $this->query("SELECT url, MAX(time_taken) AS maximum_time_taken FROM tiki_performance WHERE url LIKE ? GROUP BY url ORDER BY maximum_time_taken $order", ["%$find%"], $amount, $offset);
     }
 
     /**


=====================================
tiki-performance_stats.php
=====================================
@@ -36,46 +36,26 @@ $find = $_REQUEST['find'] ?? '';
 $averageStatOffset = $_REQUEST['average_stat_offset'] ?? 0;
 $maximumStatOffset = $_REQUEST['maximum_stat_offset'] ?? 0;
 $detailsUrl = $_REQUEST['details_url'] ?? '';
-
-/**
- * Validates a sort direction ('ASC' or 'DESC').
- * Falls back to a default if invalid, and provides feedback on invalid input.
- */
-function validateDirection(?string $direction, string $default = 'DESC'): string
-{
-    $allowedDirections = ['ASC', 'DESC'];
-
-    if (! is_string($direction)) {
-        return $default;
-    }
-
-    $normalized = strtoupper($direction);
-
-    if (in_array($normalized, $allowedDirections, true)) {
-        return $normalized;
-    }
-
-    // Detect and report invalid input that is not null, not a string, or not an allowed value.
-    Feedback::warning(tra(
-        'Invalid sort direction provided. ' . $direction . ' only ASC or DESC is allowed'
-    ));
-
-    return $default;
+$maximumStatOrder = $_REQUEST['maximum_stat_order'] ?? 'DESC';
+$maximumStatOrder = strtoupper($maximumStatOrder);
+if (! in_array($maximumStatOrder, ['ASC', 'DESC'])) {
+    $maximumStatOrder = 'DESC';
 }
-
-// Determine the order type and the requested direction.
-if (! empty($_REQUEST['no_of_requests'])) {
+$averageStatOrder = $_REQUEST['average_stat_order'] ?? 'DESC';
+$averageStatOrder = strtoupper($averageStatOrder);
+if (! in_array($averageStatOrder, ['ASC', 'DESC'])) {
+    $averageStatOrder = 'DESC';
+}
+$orderType = 'average_stat_order';
+$noOfRequests = $_REQUEST['no_of_requests'] ?? '';
+if (! empty($noOfRequests)) {
+    $noOfRequests = strtoupper($noOfRequests);
+    $averageStatOrder = in_array($noOfRequests, ['ASC', 'DESC']) ? $noOfRequests : 'DESC';
     $orderType = 'no_of_requests';
-    $requestedAverageOrder = $_REQUEST['no_of_requests'];
 } else {
     $orderType = 'average_stat_order';
-    $requestedAverageOrder = $_REQUEST['average_stat_order'] ?? null;
 }
 
-// Validate directions
-$averageStatOrder = validateDirection($requestedAverageOrder);
-$maximumStatOrder = validateDirection($_REQUEST['maximum_stat_order'] ?? null);
-
 $smarty->assign('performance_stats_lib', $performanceLib);
 $smarty->assign('find', $find);
 $smarty->assign('pages_count', $performanceLib->getRequestsGroupedByAmount($find));



View it on GitLab: https://gitlab.com/tikiwiki/tiki/-/commit/94fd808f546479ff64ca1a524d5101a6f6893fbe

-- 
View it on GitLab: https://gitlab.com/tikiwiki/tiki/-/commit/94fd808f546479ff64ca1a524d5101a6f6893fbe
You're receiving this email because of your account on gitlab.com. Manage all notifications: https://gitlab.com/-/profile/notifications | Help: https://gitlab.com/help

_______________________________________________
TikiWiki-cvs mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/tikiwiki-cvs