From d3c7e16a00772a8d86e7a3d47e0f18c314aec0d7 Mon Sep 17 00:00:00 2001 From: Roly Rudy Gutierrez Pinto Date: Fri, 23 Apr 2021 00:17:54 -0400 Subject: [PATCH] PMCORE-1225 execute-query-blacklist.ini not working according to the documentation --- workflow/engine/classes/class.pmFunctions.php | 134 +++++------------- .../engine/src/ProcessMaker/Core/System.php | 1 + .../ProcessMaker/Validation/SqlBlacklist.php | 124 ++++++++++++++++ 3 files changed, 157 insertions(+), 102 deletions(-) create mode 100644 workflow/engine/src/ProcessMaker/Validation/SqlBlacklist.php diff --git a/workflow/engine/classes/class.pmFunctions.php b/workflow/engine/classes/class.pmFunctions.php index ef0561cf5..d90073fdf 100644 --- a/workflow/engine/classes/class.pmFunctions.php +++ b/workflow/engine/classes/class.pmFunctions.php @@ -1,38 +1,11 @@ . - * - * 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 - foreach ($parseSqlStm as $key => $value) { - if($key === 'parsed'){ - $aParseSqlStm = $value; - continue; - } - } - $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))); - } - } - } - } - } + try { + (new SqlBlacklist($sqlStatement))->validate(); + } catch (Exception $e) { + G::SendTemporalMessage($e->getMessage(), 'error', 'labels'); + throw new SQLException($e->getMessage()); } - $statement = trim( $SqlStatement ); - $statement = str_replace( '(', '', $statement ); + $statement = trim($sqlStatement); + $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 ): - $result = $con->select( $SqlStatement ); + case preg_match("/^(SELECT|EXECUTE|EXEC|SHOW|DESCRIBE|EXPLAIN|BEGIN)\s/i", $statement): + $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 ): - $result = $con->insert( $SqlStatement ); + case preg_match("/^INSERT\s/i", $statement): + $result = $con->insert($sqlStatement); break; - case preg_match( "/^REPLACE\s/i", $statement ): - $result = $con->update( $SqlStatement ); + case preg_match("/^REPLACE\s/i", $statement): + $result = $con->update($sqlStatement); break; - case preg_match( "/^UPDATE\s/i", $statement ): - $result = $con->update( $SqlStatement ); + case preg_match("/^UPDATE\s/i", $statement): + $result = $con->update($sqlStatement); break; - case preg_match( "/^DELETE\s/i", $statement ): - $result = $con->delete( $SqlStatement ); + case preg_match("/^DELETE\s/i", $statement): + $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', diff --git a/workflow/engine/src/ProcessMaker/Core/System.php b/workflow/engine/src/ProcessMaker/Core/System.php index 617ccac45..87c838d15 100644 --- a/workflow/engine/src/ProcessMaker/Core/System.php +++ b/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 = '') { diff --git a/workflow/engine/src/ProcessMaker/Validation/SqlBlacklist.php b/workflow/engine/src/ProcessMaker/Validation/SqlBlacklist.php new file mode 100644 index 000000000..a8777ee67 --- /dev/null +++ b/workflow/engine/src/ProcessMaker/Validation/SqlBlacklist.php @@ -0,0 +1,124 @@ +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); + } +}