PMCORE-1225 execute-query-blacklist.ini not working according to the documentation

workflow/engine/classes/class.pmFunctions.php

@@ -1,38 +1,11 @@
 <?php
-/**
+
- * class.pmFunctions.php
- *
- * ProcessMaker Open Source Edition
- * Copyright (C) 2004 - 2008 Colosa Inc.23
- *
- * This program is free software: you can redistribute it and/or modify
- * it under the terms of the GNU Affero General Public License as
- * published by the Free Software Foundation, either version 3 of the
- * License, or (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU Affero General Public License for more details.
- *
- * You should have received a copy of the GNU Affero General Public License
- * along with this program. If not, see <http://www.gnu.org/licenses/>.
- *
- * For more information, contact Colosa Inc, 2566 Le Jeune Rd.,
- * Coral Gables, FL, 33134, USA, or email info@colosa.com.
- */
-////////////////////////////////////////////////////
-// PM Functions
-//
-// Copyright (C) 2007 COLOSA
-//
-// License: LGPL, see LICENSE
-////////////////////////////////////////////////////
 use Illuminate\Support\Facades\Log;
 use ProcessMaker\BusinessModel\Cases as BusinessModelCases;
 use ProcessMaker\Core\System;
 use ProcessMaker\Plugins\PluginRegistry;
 use ProcessMaker\Util\ElementTranslation;
+use ProcessMaker\Validation\SqlBlacklist;
 use Illuminate\Support\Facades\DB;
 
 /**
@@ -241,101 +214,60 @@ function literalDate ($date, $lang = 'en')
  * @throws SQLException
  *
  */
-function executeQuery ($SqlStatement, $DBConnectionUID = 'workflow', $aParameter = array())
+function executeQuery($sqlStatement, $dbConnectionUID = 'workflow', $parameters = [])
 {
     // This means the DBConnectionUID is not loaded yet, so we'll force DbConnections::loadAdditionalConnections
-    if (is_null(config('database.connections.' . $DBConnectionUID . '.driver'))) {
+    if (is_null(config('database.connections.' . $dbConnectionUID . '.driver'))) {
         // Force to load the external connections
         DbConnections::loadAdditionalConnections();
     }
 
-    if (config('database.connections.' . $DBConnectionUID . '.driver') !== 'oracle') {
+    if (config('database.connections.' . $dbConnectionUID . '.driver') !== 'oracle') {
         // If the connections drivers are "mysql", "pgsql" or "sqlsrv" we're using Laravel
-        $con = DB::connection($DBConnectionUID);
+        $con = DB::connection($dbConnectionUID);
         $con->beginTransaction();
     } else {
         // If the connection driver is "oracle" we are using the native oci8 functions
-        $con = Propel::getConnection($DBConnectionUID);
+        $con = Propel::getConnection($dbConnectionUID);
         $con->begin();
     }
     
-    $blackList = System::getQueryBlackList();
-    $listQueries = explode('|', isset($blackList['queries']) ? $blackList['queries'] : '');
-    $aListAllTables = explode(
-        '|',
-        ((isset($blackList['tables']))? $blackList['tables'] : '') .
-        ((isset($blackList['pmtables']))? $blackList['pmtables'] : '')
-    );
-    $parseSqlStm = new PHPSQLParser($SqlStatement);
     try {
-        //Parsing queries and check the blacklist
+        try {
-        foreach ($parseSqlStm as $key => $value) {
+            (new SqlBlacklist($sqlStatement))->validate();
-            if($key === 'parsed'){
+        } catch (Exception $e) {
-                $aParseSqlStm = $value;
+            G::SendTemporalMessage($e->getMessage(), 'error', 'labels');
-                continue;
+            throw new SQLException($e->getMessage());
-            }
-        }
-        $nameOfTable = '';
-        $arrayOfTables = array();
-        foreach ($aParseSqlStm as $key => $value) {
-            if(in_array($key, $listQueries)){
-                if(isset($value['table'])){
-                    $nameOfTable = $value['table'];
-                } else {
-                    foreach ($value as $valueTab) {
-                        if(is_array($valueTab)){
-                            $arrayOfTables = $valueTab;
-                        } else {
-                            $nameOfTable = $valueTab;
-                        }
-                    }
-                }
-                if(isset($nameOfTable) && $nameOfTable !== ''){
-                    if(in_array($nameOfTable,$aListAllTables)){
-                        G::SendTemporalMessage( G::loadTranslation('ID_NOT_EXECUTE_QUERY', array($nameOfTable)), 'error', 'labels' );
-                        throw new SQLException(G::loadTranslation('ID_NOT_EXECUTE_QUERY', array($nameOfTable)));
-                    }
-                }
-                if (is_array($arrayOfTables)){
-                    foreach ($arrayOfTables as $row){
-                        if(!empty($row)){
-                            if(in_array($row, $aListAllTables)){
-                                G::SendTemporalMessage(G::loadTranslation('ID_NOT_EXECUTE_QUERY', array($nameOfTable)), 'error', 'labels' );
-                                throw new SQLException(G::loadTranslation('ID_NOT_EXECUTE_QUERY', array($nameOfTable)));
-                            }
-                        }
-                    }
-                }
-            }
         }
 
-        $statement = trim( $SqlStatement );
+        $statement = trim($sqlStatement);
-        $statement = str_replace( '(', '', $statement );
+        $statement = str_replace('(', '', $statement);
 
         $result = false;
-
         // Check to see if we're not running oracle, which is usually a safe default
-        if (config('database.connections.' . $DBConnectionUID . '.driver') != 'oracle') {
+        if (config('database.connections.' . $dbConnectionUID . '.driver') != 'oracle') {
             try {
                 switch (true) {
-                    case preg_match( "/^(SELECT|EXECUTE|EXEC|SHOW|DESCRIBE|EXPLAIN|BEGIN)\s/i", $statement ):
+                    case preg_match("/^(SELECT|EXECUTE|EXEC|SHOW|DESCRIBE|EXPLAIN|BEGIN)\s/i", $statement):
-                        $result = $con->select( $SqlStatement );
+                        $result = $con->select($sqlStatement);
                         // Convert to 1 index key array of array results
-                        $result = collect($result)->map(function($x) { return (array)$x; })->toArray();
+                        $result = collect($result)->map(function ($x) {
+                                return (array) $x;
+                            })->toArray();
                         array_unshift($result, []);
                         unset($result[0]);
                         break;
-                    case preg_match( "/^INSERT\s/i", $statement ):
+                    case preg_match("/^INSERT\s/i", $statement):
-                        $result = $con->insert( $SqlStatement );
+                        $result = $con->insert($sqlStatement);
                         break;
-                    case preg_match( "/^REPLACE\s/i", $statement ):
+                    case preg_match("/^REPLACE\s/i", $statement):
-                        $result = $con->update( $SqlStatement );
+                        $result = $con->update($sqlStatement);
                         break;
-                    case preg_match( "/^UPDATE\s/i", $statement ):
+                    case preg_match("/^UPDATE\s/i", $statement):
-                        $result = $con->update( $SqlStatement );
+                        $result = $con->update($sqlStatement);
                         break;
-                    case preg_match( "/^DELETE\s/i", $statement ):
+                    case preg_match("/^DELETE\s/i", $statement):
-                        $result = $con->delete( $SqlStatement );
+                        $result = $con->delete($sqlStatement);
                         break;
                 }
                 $con->commit();
@@ -347,21 +279,19 @@ function executeQuery ($SqlStatement, $DBConnectionUID = 'workflow', $aParameter
             $dataEncode = $con->getDSN();
 
             if (isset($dataEncode["encoding"]) && $dataEncode["encoding"] != "") {
-                $result = executeQueryOci($SqlStatement, $con, $aParameter, $dataEncode["encoding"]);
+                $result = executeQueryOci($sqlStatement, $con, $parameters, $dataEncode["encoding"]);
             } else {
-                $result = executeQueryOci($SqlStatement, $con, $aParameter);
+                $result = executeQueryOci($sqlStatement, $con, $parameters);
             }
         }
-        //Logger
         $message = 'Sql Execution';
         $context = [
             'action' => 'execute-query',
-            'sql' => $SqlStatement
+            'sql' => $sqlStatement
         ];
         Log::channel(':sqlExecution')->info($message, Bootstrap::context($context));
         return $result;
     } catch (SQLException $sqle) {
-        //Logger
         $message = 'Sql Execution';
         $context = [
             'action' => 'execute-query',

workflow/engine/src/ProcessMaker/Core/System.php

@@ -1253,6 +1253,7 @@ class System
      * @access public
      * @param string $globalIniFile
      * @return array of execute query Black list
+     * @deprecated since version 3.6.4
      */
     public static function getQueryBlackList($globalIniFile = '')
     {

workflow/engine/src/ProcessMaker/Validation/SqlBlacklist.php

@@ -0,0 +1,124 @@
+<?php
+
+namespace ProcessMaker\Validation;
+
+use Exception;
+use G;
+use PhpMyAdmin\SqlParser\Parser;
+
+class SqlBlacklist extends Parser
+{
+
+    /**
+     * Define the statements to block, this is case sensitive.
+     * @var array
+     */
+    private $statementsToBeBlocked = [
+        'SELECT',
+        'EXECUTE',
+        'EXEC',
+        'SHOW',
+        'DESCRIBE',
+        'EXPLAIN',
+        'BEGIN',
+        'INSERT',
+        'UPDATE',
+        'DELETE',
+        'REPLACE'
+    ];
+
+    /**
+     * Constructor of class.
+     * @param string $list
+     * @param boolean $strict
+     */
+    public function __construct($list = null, $strict = false)
+    {
+        parent::__construct($list, $strict);
+    }
+
+    /**
+     * Get information about the statements permitted and tables that can be modified.
+     * @return array
+     */
+    public function getConfigValues(): array
+    {
+        $tables = [];
+        $statements = [];
+        $pmtables = [];
+
+        $path = PATH_CONFIG . 'system-tables.ini';
+        if (file_exists($path)) {
+            $values = @parse_ini_file($path);
+
+            $string = isset($values['tables']) ? $values['tables'] : '';
+            $tables = explode('|', $string);
+            $tables = array_filter($tables, function ($v) {
+                return !empty($v);
+            });
+        }
+
+        $path = PATH_CONFIG . 'execute-query-blacklist.ini';
+        if (file_exists($path)) {
+            $values = @parse_ini_file($path);
+
+            $string = isset($values['pmtables']) ? $values['pmtables'] : '';
+            $pmtables = explode('|', $string);
+            $pmtables = array_filter($pmtables, function ($v) {
+                return !empty($v);
+            });
+
+            $string = isset($values['queries']) ? $values['queries'] : '';
+            $string = strtoupper($string);
+            $statements = explode('|', $string);
+            //get only statements allowed for lock
+            $statements = array_filter($statements, function ($v) {
+                $toUpper = strtoupper($v);
+                return !empty($v) && in_array($toUpper, $this->statementsToBeBlocked);
+            });
+        }
+
+        return [
+            'tables' => $tables,
+            'statements' => $statements,
+            'pmtables' => $pmtables
+        ];
+    }
+
+    /**
+     * Parse a sql string and check the blacklist, an exception is thrown if it contains a restricted item.
+     * @return void
+     * @throws Exception
+     */
+    public function validate(): void
+    {
+        $config = $this->getConfigValues();
+
+        //verify statements
+        foreach ($this->statements as $statement) {
+            $signed = get_class($statement);
+            foreach (Parser::$STATEMENT_PARSERS as $key => $value) {
+                if ($signed === $value && in_array(strtoupper($key), $config['statements'])) {
+                    throw new Exception(G::loadTranslation('ID_INVALID_QUERY'));
+                }
+            }
+        }
+
+        //verify tables
+        //tokens are formed multidimensionally, it is necessary to recursively traverse the multidimensional object.
+        $listTables = array_merge($config['tables'], $config['pmtables']);
+        $fn = function ($object) use (&$fn, $listTables) {
+            foreach ($object as $key => $value) {
+                if (is_array($value) || is_object($value)) {
+                    $fn($value);
+                }
+                if ($key === 'table' && is_string($value)) {
+                    if (in_array($value, $listTables)) {
+                        throw new Exception(G::loadTranslation('ID_NOT_EXECUTE_QUERY', [$value]));
+                    }
+                }
+            }
+        };
+        $fn($this->statements);
+    }
+}