[TikiWiki-commits] [Git][tikiwiki/tiki][24.x] [FIX] filegallery: block shell metacharacters in indexing handlers and add ...

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


Commits:
7ff7ee04 by Alfred Syatsukwa at 2026-08-18T08:02:02+00:00
[FIX] filegallery: block shell metacharacters in indexing handlers and add  CSRF check to reindex trigger
---
* [FIX] filegallery: block shell metacharacters in indexing handlers and add add CSRF check to reindex trigger
---

See merge request tikiwiki/tiki!10926

(cherry picked from commit 6c7b73f6803df2e5822e1884d3d4ab348367d49f)

See merge request tikiwiki/tiki!10927

- - - - -


2 changed files:

- admin/include_fgal.php
- lib/filegals/filegallib.php


Changes:

=====================================
admin/include_fgal.php
=====================================
@@ -72,14 +72,16 @@ if (! empty($_POST['mimes']) && $access->checkCsrf()) {
         $mime = trim($mime);
         if (empty($cmd)) {
             $filegallib->delete_file_handler($mime);
-        } else {
-            $filegallib->change_file_handler($mime, $cmd);
+        } elseif (! $filegallib->change_file_handler($mime, $cmd)) {
+            Feedback::error(tr('Invalid file handler command for MIME type "%0". Only known extractor binaries are allowed, and shell metacharacters are not permitted.', $mime));
         }
     }
 }
 
 if (! empty($_POST['newMime']) && ! empty($_POST['newCmd']) && $access->checkCsrf()) {
-    $filegallib->change_file_handler($_POST['newMime'], $_POST['newCmd']);
+    if (! $filegallib->change_file_handler($_POST['newMime'], $_POST['newCmd'])) {
+        Feedback::error(tr('Invalid file handler command for MIME type "%0". Only known extractor binaries are allowed, and shell metacharacters are not permitted.', $_POST['newMime']));
+    }
 }
 
 if (isset($_REQUEST["filegalfixvndmsfiles"]) && $access->checkCsrf()) {
@@ -88,7 +90,7 @@ if (isset($_REQUEST["filegalfixvndmsfiles"]) && $access->checkCsrf()) {
 
 //*** end state-changing actions
 
-if (isset($_REQUEST["filegalredosearch"])) {
+if (isset($_REQUEST["filegalredosearch"]) && $access->checkCsrf()) {
     $searchTextReindexedFilesAmount = $filegallib->reindex_all_files_for_search_text();
     Feedback::success(tr("The search text was reindexed for a total of %0 files.", $searchTextReindexedFilesAmount));
 }


=====================================
lib/filegals/filegallib.php
=====================================
@@ -871,6 +871,10 @@ class FileGalLib extends TikiLib
 
     public function change_file_handler($mime_type, $cmd)
     {
+        if (! $this->isSafeHandlerCommand($cmd)) {
+            return false;
+        }
+
         $handlers = $this->table('tiki_file_handlers');
 
         $mime_type = trim($mime_type);
@@ -905,38 +909,72 @@ class FileGalLib extends TikiLib
         }
     }
 
+    /**
+     * Built-in MIME → candidate handler command templates.
+     * Partially duplicated in tiki-check.php for standalone mode checks.
+     */
+    private function getHandlerCommandPossibilities()
+    {
+        return [
+            'application/ms-excel' => ['xls2csv %1'],
+            'application/msexcel' => ['xls2csv %1'],
+            'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet' => ['xlsx2csv.py %1'],
+            'application/ms-powerpoint' => ['catppt %1'],
+            'application/mspowerpoint' => ['catppt %1'],
+            'application/vnd.openxmlformats-officedocument.presentationml.presentation' => ['pptx2txt.pl %1 -'],
+            'application/msword' => ['catdoc %1', 'strings %1'],
+            'application/vnd.openxmlformats-officedocument.wordprocessingml.document' => ['docx2txt.pl %1 -'],
+            'application/pdf' => ['pstotext %1', 'pdftotext %1 -'],
+            'application/postscript' => ['pstotext %1'],
+            'application/ps' => ['pstotext %1'],
+            'application/rtf' => ['catdoc %1'],
+            'application/sgml' => ['col -b %1', 'strings %1'],
+            'application/vnd.ms-excel' => ['xls2csv %1'],
+            'application/vnd.ms-powerpoint' => ['catppt %1'],
+            'application/x-msexcel' => ['xls2csv %1'],
+            'application/x-pdf' => ['pstotext %1', 'pdftotext %1 -'],
+            'application/x-troff-man' => ['man -l %1'],
+            'application/zip' => ['unzip -l %1'],
+            'text/enriched' => ['col -b %1', 'strings %1'],
+            'text/html' => ['elinks -dump -no-home %1'],
+            'text/richtext' => ['col -b %1', 'strings %1'],
+            'text/sgml' => ['col -b %1', 'strings %1'],
+            'text/tab-separated-values' => ['col -b %1', 'strings %1'],
+        ];
+    }
+
+    /**
+     * Binary basenames allowed in file-handler command templates.
+     * Derived from the built-in possibilities list.
+     */
+    private function getAllowedHandlerBinaries()
+    {
+        static $allowed;
+
+        if ($allowed !== null) {
+            return $allowed;
+        }
+
+        $allowed = [];
+        foreach ($this->getHandlerCommandPossibilities() as $options) {
+            foreach ($options as $opt) {
+                $optArray = explode(' ', $opt, 2);
+                $exec = reset($optArray);
+                if ($exec !== false && $exec !== '') {
+                    $allowed[$exec] = true;
+                }
+            }
+        }
+
+        return $allowed;
+    }
+
     public function get_file_handlers($for_execution = false)
     {
         $cachelib = TikiLib::lib('cache');
 
         if ($for_execution && ! $default = $cachelib->getSerialized('file_handlers')) {
-            // n.b. this array is partially duplicated in tiki-check.php for standalone mode checks
-            $possibilities = [
-                'application/ms-excel' => ['xls2csv %1'],
-                'application/msexcel' => ['xls2csv %1'],
-                'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet' => ['xlsx2csv.py %1'],
-                'application/ms-powerpoint' => ['catppt %1'],
-                'application/mspowerpoint' => ['catppt %1'],
-                'application/vnd.openxmlformats-officedocument.presentationml.presentation' => ['pptx2txt.pl %1 -'],
-                'application/msword' => ['catdoc %1', 'strings %1'],
-                'application/vnd.openxmlformats-officedocument.wordprocessingml.document' => ['docx2txt.pl %1 -'],
-                'application/pdf' => ['pstotext %1', 'pdftotext %1 -'],
-                'application/postscript' => ['pstotext %1'],
-                'application/ps' => ['pstotext %1'],
-                'application/rtf' => ['catdoc %1'],
-                'application/sgml' => ['col -b %1', 'strings %1'],
-                'application/vnd.ms-excel' => ['xls2csv %1'],
-                'application/vnd.ms-powerpoint' => ['catppt %1'],
-                'application/x-msexcel' => ['xls2csv %1'],
-                'application/x-pdf' => ['pstotext %1', 'pdftotext %1 -'],
-                'application/x-troff-man' => ['man -l %1'],
-                'application/zip' => ['unzip -l %1'],
-                'text/enriched' => ['col -b %1', 'strings %1'],
-                'text/html' => ['elinks -dump -no-home %1'],
-                'text/richtext' => ['col -b %1', 'strings %1'],
-                'text/sgml' => ['col -b %1', 'strings %1'],
-                'text/tab-separated-values' => ['col -b %1', 'strings %1'],
-            ];
+            $possibilities = $this->getHandlerCommandPossibilities();
 
             $default = [];
             $executables = [];
@@ -1035,6 +1073,10 @@ class FileGalLib extends TikiLib
         }
 
         return function (FileWrapper $wrapper) use ($command) {
+            if (! $this->isSafeHandlerCommand($command)) {
+                return false;
+            }
+
             $tmpfname = $wrapper->getReadableFile();
 
             $cmd = str_replace('%1', escapeshellarg($tmpfname), $command);
@@ -1051,6 +1093,60 @@ class FileGalLib extends TikiLib
         };
     }
 
+    /**
+     * Ensure a handler template is safe to pass to popen()/`/bin/sh -c`.
+     *
+     * The binary (first token, basename) must be one of the built-in
+     * extractors from getHandlerCommandPossibilities(). Absolute paths are
+     * allowed (including Windows paths with '\' and drive letters) as long as
+     * the basename matches. Shell metacharacters are rejected so DB-stored
+     * overrides cannot inject extra commands. The %1 file path is still
+     * escaped separately via escapeshellarg().
+     */
+    private function isSafeHandlerCommand($command)
+    {
+        $command = trim($command);
+        if ($command === '') {
+            return false;
+        }
+
+        // Deny shell control characters; allow '\' and ':' for Windows paths,
+        // and '=' / '~' for common tool flags and home-relative paths.
+        if (preg_match('/[;|&$`<>()\'"!\r\n]/', $command)) {
+            return false;
+        }
+
+        return $this->isAllowedHandlerBinary($this->getHandlerCommandBinary($command));
+    }
+
+    private function getHandlerCommandBinary($command)
+    {
+        $parts = preg_split('/\s+/', trim($command), 2);
+        $binary = $parts[0] ?? '';
+
+        return basename(str_replace('\\', '/', $binary));
+    }
+
+    private function isAllowedHandlerBinary($basename)
+    {
+        if ($basename === '') {
+            return false;
+        }
+
+        $allowed = $this->getAllowedHandlerBinaries();
+        if (isset($allowed[$basename])) {
+            return true;
+        }
+
+        // Windows executables: pdftotext.exe → pdftotext
+        if (preg_match('/\.(exe|bat|cmd)$/i', $basename)) {
+            $withoutExt = preg_replace('/\.(exe|bat|cmd)$/i', '', $basename);
+            return isset($allowed[$withoutExt]);
+        }
+
+        return false;
+    }
+
     public function get_search_text_for_data($file)
     {
         $parseApp = $this->get_parse_app($file->filetype);
@@ -1829,7 +1925,8 @@ class FileGalLib extends TikiLib
      * This is useful if user doesn't have permissions to download files in the corresponding file
      * gallery but still can view the file through the tracker item.
      */
-    public function isBacklinkedFromAViewableTrackerItem($fileId) {
+    public function isBacklinkedFromAViewableTrackerItem($fileId)
+    {
         $objects = $this->getFileBacklinks($fileId);
         foreach ($objects as $object) {
             if ($object['type'] == 'trackeritem') {



View it on GitLab: https://gitlab.com/tikiwiki/tiki/-/commit/7ff7ee04f14cb556355c1b6b27a5e5ae07896076

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