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

Alfred Syatsukwa pushed to branch 30.x at Tiki Wiki CMS Groupware / Tiki


Commits:
25124090 by Alfred Syatsukwa at 2026-08-18T17:43:39+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
---

See merge request tikiwiki/tiki!10955

(cherry picked from commit 94fd808f546479ff64ca1a524d5101a6f6893fbe)

See merge request tikiwiki/tiki!10957

- - - - -


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
=====================================
@@ -3060,10 +3060,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,8 +987,10 @@ 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];
@@ -996,7 +998,7 @@ class Tracker_Query
             print_r($tikilib->fetchAll($query, $params));
             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
=====================================
@@ -48,10 +48,14 @@ class PerformanceStatsLib extends TikiLib
      */
     public function getRequestsBasedOnAverageRequestTime(int $amount = 25, int $offset = 0, string $find = '', string $order = 'DESC', string $orderType = '')
     {
+        $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);
         }
     }
 
@@ -65,7 +69,11 @@ class PerformanceStatsLib extends TikiLib
      */
     public function getRequestsBasedOnMaximumProcessingTime(int $amount = 25, int $offset = 0, string $find = '', string $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 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
=====================================
@@ -34,12 +34,25 @@ if (! empty($_REQUEST['clear']) && $access->checkCsrf()) {
 $find = $_REQUEST['find'] ?? '';
 $averageStatOffset = $_REQUEST['average_stat_offset'] ?? 0;
 $maximumStatOffset = $_REQUEST['maximum_stat_offset'] ?? 0;
+$detailsUrl = $_REQUEST['details_url'] ?? '';
 $maximumStatOrder = $_REQUEST['maximum_stat_order'] ?? 'DESC';
+$maximumStatOrder = strtoupper($maximumStatOrder);
+if (! in_array($maximumStatOrder, ['ASC', 'DESC'])) {
+    $maximumStatOrder = 'DESC';
+}
 $averageStatOrder = $_REQUEST['average_stat_order'] ?? 'DESC';
+$averageStatOrder = strtoupper($averageStatOrder);
+if (! in_array($averageStatOrder, ['ASC', 'DESC'])) {
+    $averageStatOrder = 'DESC';
+}
 $orderType = 'average_stat_order';
-if (! empty($_REQUEST['no_of_requests'])) {
-    $averageStatOrder = $_REQUEST['no_of_requests'];
+$noOfRequests = $_REQUEST['no_of_requests'] ?? '';
+if (! empty($noOfRequests)) {
+    $noOfRequests = strtoupper($noOfRequests);
+    $averageStatOrder = in_array($noOfRequests, ['ASC', 'DESC']) ? $noOfRequests : 'DESC';
     $orderType = 'no_of_requests';
+} else {
+    $orderType = 'average_stat_order';
 }
 
 $smarty->assign('performance_stats_lib', $performanceLib);



View it on GitLab: https://gitlab.com/tikiwiki/tiki/-/commit/251240903e35004c621c7680852384a723368fa9

-- 
View it on GitLab: https://gitlab.com/tikiwiki/tiki/-/commit/251240903e35004c621c7680852384a723368fa9
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