From 73bcaace5335bbe885824dcb25c70855393730af Mon Sep 17 00:00:00 2001
From: Lee Garner <lee@leegarner.com>
Date: Mon, 22 Aug 2022 08:57:05 -0700
Subject: [PATCH 1/2] Fix #555, limit to uploadable categories on file upload
 form

---
 .../filemgmt/classes/Category.class.php       | 95 +++++++++++++------
 .../filemgmt/classes/Download.class.php       |  2 +-
 2 files changed, 66 insertions(+), 31 deletions(-)

diff --git a/private/plugins/filemgmt/classes/Category.class.php b/private/plugins/filemgmt/classes/Category.class.php
index 92702a8a1..e35698818 100644
--- a/private/plugins/filemgmt/classes/Category.class.php
+++ b/private/plugins/filemgmt/classes/Category.class.php
@@ -113,16 +113,23 @@ public function Read($id = 0)
             return;
         }
 
-        $sql = "SELECT * FROM {$_TABLES['filemgmt_cat']}
-                WHERE cid = '$id'";
-        $result = DB_query($sql);
-        if (!$result || DB_numRows($result) != 1) {
-            return false;
-        } else {
-            $row = DB_fetchArray($result, false);
+        try {
+            $row = Database::getInstance()->conn->executeQuery(
+                "SELECT * FROM {$_TABLES['filemgmt_cat']}
+                WHERE cid = '$id'",
+                array($id),
+                array(Database::INTEGER)
+            )->fetchAssociative();
+        } catch (\Throwable $e) {
+            Log::write('system', Log::ERROR, __METHOD__ . ': ' . $e->getMessage());
+            $row = false;
+        }
+        if (is_array($row)) {
             $this->setVars($row, true);
             $this->isNew = false;
             return true;
+        } else {
+            return false;
         }
     }
 
@@ -710,24 +717,42 @@ public static function getAll()
     }
 
 
-    public static function getChildren($pid=0, $checkAccess=true)
+    /**
+     * Get all the child categories from a given root.
+     *
+     * @param   integer $pid    Root category ID
+     * @param   integer $checkAccess    1 to check view, 2 to check write
+     * @return  array       Array of categories
+     */
+    public static function getChildren(int $pid=0, int $checkAccess=1) : array
     {
         global $_TABLES, $_GROUPS;
 
         $retval = array();
         $pid = (int)$pid;
-        $sql = "SELECT * FROM {$_TABLES['filemgmt_cat']} WHERE pid = $pid ";
+        $sql = "SELECT * FROM {$_TABLES['filemgmt_cat']} WHERE pid = ? ";
+        $values = array($pid);
+        $types = array(Database::INTEGER);
         if ($checkAccess) {
-            if (count($_GROUPS) == 1) {
-                $sql .= " AND grp_access = '" . current($_GROUPS) ."' ";
+            $values[] = array_values($_GROUPS);
+            $types[] = Database::PARAM_INT_ARRAY;
+            if ($checkAccess == 2) {
+                $sql .= " AND grp_writeaccess IN (?) ";
             } else {
-                $sql .= " AND grp_access IN (" . implode(',',array_values($_GROUPS)) .") ";
+                $sql .= " AND grp_access IN (?) ";
             }
         }
         $sql .= "ORDER BY cid";
-        $result = DB_query($sql);
-        while ($A = DB_fetchArray($result, false)) {
-            $retval[$A['cid']] = new self($A);
+        try {
+            $stmt = Database::getInstance()->conn->executeQuery($sql, $values, $types);
+        } catch (\Throwable $e) {
+            Log::write('system', Log::ERROR, __METHOD__ . ': ' . $e->getMessage());
+            $stmt = false;
+        }
+        if ($stmt) {
+            while ($A = $stmt->fetchAssociative()) {
+                $retval[$A['cid']] = new self($A);
+            }
         }
         return $retval;
     }
@@ -785,7 +810,7 @@ public function getDscp()
      * @param   integer $current_cat    Current category ID, to set "selected"
      * @return  string      HTML for selection options
      */
-    public static function getChildOptions($pid, $indent, $current_cat)
+    public static function getChildOptions(int $pid, string $indent, int $current_cat) : string
     {
         global $_TABLES;
 
@@ -793,21 +818,31 @@ public static function getChildOptions($pid, $indent, $current_cat)
         $retval = '';
         $spaces = ($indent+1) * 2;
 
-        $sql = "SELECT * FROM {$_TABLES['filemgmt_cat']}
-            WHERE pid = $pid
-            ORDER BY title ASC";
-        $result = DB_query($sql);
-        while (($C = DB_fetchArray($result)) != NULL) {
-            $retval .= '<option value="'.$C['cid'].'"';
-            if ( $C['cid'] == $current_cat ) {
-                $retval .= ' selected="selected"';
-            }
-            $retval .= '>';
-            for ($x=0;$x<=$spaces;$x++) {
-                $retval .= '&nbsp;';
+        try {
+            $stmt = Database::getInstance()->conn->executeQuery(
+                "SELECT * FROM {$_TABLES['filemgmt_cat']}
+                WHERE pid = ?
+                ORDER BY title ASC",
+                array($pid),
+                array(Database::INTEGER)
+            );
+        } catch (\Throwable $e) {
+            Log::write('system', Log::ERROR, __METHOD__ . ': ' . $e->getMessage());
+            $stmt = false;
+        }
+        if ($stmt) {
+            while ($C = $stmt->fetchAssociative()) {
+                $retval .= '<option value="'.$C['cid'].'"';
+                if ( $C['cid'] == $current_cat ) {
+                    $retval .= ' selected="selected"';
+                }
+                $retval .= '>';
+                for ($x=0;$x<=$spaces;$x++) {
+                    $retval .= '&nbsp;';
+                }
+                $retval .= $C['title'].'</option>';
+                $retval .= self::getChildOptions($C['cid'], $indent+1, $current_cat);
             }
-            $retval .= $C['title'].'</option>';
-            $retval .= self::getChildOptions($C['cid'], $indent+1, $current_cat);
         }
         return $retval;
     }
diff --git a/private/plugins/filemgmt/classes/Download.class.php b/private/plugins/filemgmt/classes/Download.class.php
index 406c73796..2def1ae5f 100644
--- a/private/plugins/filemgmt/classes/Download.class.php
+++ b/private/plugins/filemgmt/classes/Download.class.php
@@ -1018,7 +1018,7 @@ public function edit($post=array())
         $pathstring .= "<a href=\"{$_FM_CONF['url']}/index.php?id={$this->lid}\">{$hdr_title}</a>";
 
         $categorySelectHTML = '';
-        $rootCats = Category::getChildren(0, true);
+        $rootCats = Category::getChildren(0, 2);
         foreach ($rootCats as $cid=>$Cat) {
             $categorySelectHTML .= '<option value="'.$cid.'"';
             if ($cid == $this->cid) {

From b085b237775873e038e51a85f10e6e0761b497a7 Mon Sep 17 00:00:00 2001
From: Lee Garner <lee@leegarner.com>
Date: Thu, 25 Aug 2022 17:31:09 -0700
Subject: [PATCH 2/2] Use querybuilder for clearer SQL

---
 .../filemgmt/classes/Category.class.php       | 31 +++++++++++--------
 public_html/admin/plugins/filemgmt/index.php  |  2 +-
 2 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/private/plugins/filemgmt/classes/Category.class.php b/private/plugins/filemgmt/classes/Category.class.php
index e35698818..5ed6cbcbd 100644
--- a/private/plugins/filemgmt/classes/Category.class.php
+++ b/private/plugins/filemgmt/classes/Category.class.php
@@ -23,6 +23,9 @@
  */
 class Category
 {
+    const ACCESS_READ = 1;
+    const ACCESS_WRITE = 2;
+
     /** Category ID.
      * @var integer */
     private $cid = 0;
@@ -60,7 +63,6 @@ public function __construct($id=0)
         global $_USER, $_VARS;
 
         $this->isNew = true;
-
         if (is_array($id)) {
             $this->setVars($id, true);
         } elseif ($id > 0) {
@@ -102,7 +104,7 @@ public function setVars($row, $fromDB=false)
      * @param   integer $id Optional ID.  Current ID is used if zero.
      * @return  boolean     True if a record was read, False on failure
      */
-    public function Read($id = 0)
+    public function Read(int $id = 0) : bool
     {
         global $_TABLES;
 
@@ -110,13 +112,13 @@ public function Read($id = 0)
         if ($id == 0) $id = $this->cid;
         if ($id == 0) {
             $this->error = 'Invalid ID in Read()';
-            return;
+            return false;
         }
 
         try {
             $row = Database::getInstance()->conn->executeQuery(
                 "SELECT * FROM {$_TABLES['filemgmt_cat']}
-                WHERE cid = '$id'",
+                WHERE cid = ?",
                 array($id),
                 array(Database::INTEGER)
             )->fetchAssociative();
@@ -141,7 +143,7 @@ public function Read($id = 0)
      * @param   integer $cid    Category ID
      * @return  object          Category object
      */
-    public static function getInstance($cid)
+    public static function getInstance(int $cid)
     {
         static $cats = array();
         if (!isset($cats[$cid])) {
@@ -729,22 +731,25 @@ public static function getChildren(int $pid=0, int $checkAccess=1) : array
         global $_TABLES, $_GROUPS;
 
         $retval = array();
-        $pid = (int)$pid;
-        $sql = "SELECT * FROM {$_TABLES['filemgmt_cat']} WHERE pid = ? ";
+        $qb = Database::getInstance()->conn->createQueryBuilder();
+        $qb->select('*')
+           ->from($_TABLES['filemgmt_cat'])
+           ->where('pid = :pid')
+           ->setParameter('pid', $pid, Database::INTEGER)
+           ->orderBy('cid', 'ASC');
         $values = array($pid);
         $types = array(Database::INTEGER);
         if ($checkAccess) {
             $values[] = array_values($_GROUPS);
-            $types[] = Database::PARAM_INT_ARRAY;
-            if ($checkAccess == 2) {
-                $sql .= " AND grp_writeaccess IN (?) ";
+            if ($checkAccess == self::ACCESS_WRITE) {
+                $qb->andWhere('grp_writeaccess IN (:groups)');
             } else {
-                $sql .= " AND grp_access IN (?) ";
+                $qb->andWhere('grp_access IN (:groups)');
             }
+            $qb->setParameter('groups', array_values($_GROUPS), Database::PARAM_INT_ARRAY);
         }
-        $sql .= "ORDER BY cid";
         try {
-            $stmt = Database::getInstance()->conn->executeQuery($sql, $values, $types);
+            $stmt = $qb->execute();
         } catch (\Throwable $e) {
             Log::write('system', Log::ERROR, __METHOD__ . ': ' . $e->getMessage());
             $stmt = false;
diff --git a/public_html/admin/plugins/filemgmt/index.php b/public_html/admin/plugins/filemgmt/index.php
index e5492f048..8a3663c1d 100644
--- a/public_html/admin/plugins/filemgmt/index.php
+++ b/public_html/admin/plugins/filemgmt/index.php
@@ -168,7 +168,7 @@
     COM_refresh("{$_FM_CONF['admin_url']}/index.php?categoryConfigAdmin");
     break;
 case 'modCat':
-    $content .= Filemgmt\Category::getInstance($opval)->edit();
+    $content .= Filemgmt\Category::getInstance((int)$opval)->edit();
     break;
 case 'saveCat':
 case "modCatS":