Yaron Koren has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/227241

Change subject: Added SQL function validation for all parameters
......................................................................

Added SQL function validation for all parameters

Via the new CargoSQLQuery::getAndValidateSQLFunctions() method.

Change-Id: I1e4ac5a712e1cc55d56585615cb9ac3545b51122
---
M CargoSQLQuery.php
1 file changed, 33 insertions(+), 22 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Cargo 
refs/changes/41/227241/1

diff --git a/CargoSQLQuery.php b/CargoSQLQuery.php
index 8c1b050..d0238ae 100644
--- a/CargoSQLQuery.php
+++ b/CargoSQLQuery.php
@@ -136,6 +136,13 @@
                                throw new MWException( "Error: the string 
\"$displayString\" cannot be used within #cargo_query." );
                        }
                }
+
+               self::getAndValidateSQLFunctions( $simplifiedWhereStr );
+               self::getAndValidateSQLFunctions( $joinOnStr );
+               self::getAndValidateSQLFunctions( $groupByStr );
+               self::getAndValidateSQLFunctions( $havingStr );
+               self::getAndValidateSQLFunctions( $orderByStr );
+               self::getAndValidateSQLFunctions( $limitStr );
        }
 
        /**
@@ -332,6 +339,24 @@
                }
        }
 
+       static function getAndValidateSQLFunctions( $str ) {
+               global $wgCargoAllowedSQLFunctions;
+
+               $sqlFunctionMatches = array();
+               $sqlFunctionRegex = '/\b(\S*?)\s?\(/';
+               preg_match_all( $sqlFunctionRegex, $str, $sqlFunctionMatches );
+               $sqlFunctions = array_map( 'strtoupper', $sqlFunctionMatches[1] 
);
+               // Throw an error if any of these functions
+               // are not in our "whitelist" of SQL functions.
+               foreach ( $sqlFunctions as $sqlFunction ) {
+                       if ( !in_array( $sqlFunction, 
$wgCargoAllowedSQLFunctions ) ) {
+                               throw new MWException( "Error: the SQL function 
\"$sqlFunction()\" is not allowed." );
+                       }
+               }
+
+               return $sqlFunctions;
+       }
+
        /**
         * Attempts to get the "field description" (type, etc.) of each field
         * specified in a SELECT call (via a #cargo_query call), using the set
@@ -345,12 +370,15 @@
 
                $this->mFieldDescriptions = array();
                $this->mFieldTables = array();
-               foreach ( $this->mAliasedFieldNames as $alias => $fieldName ) {
+               foreach ( $this->mAliasedFieldNames as $alias => $origFieldName 
) {
                        $tableName = null;
-                       if ( strpos( $fieldName, '.' ) !== false ) {
+                       if ( strpos( $origFieldName, '.' ) !== false ) {
                                // This could probably be done better with
                                // regexps.
-                               list( $tableName, $fieldName ) = explode( '.', 
$fieldName, 2 );
+                               list( $tableName, $fieldName ) = explode( '.', 
$origFieldName, 2 );
+                       } else {
+                               $fieldName = $origFieldName;
+                               $tableName = null;
                        }
                        $description = new CargoFieldDescription();
                        // If it's a pre-defined field, we probably know its
@@ -359,25 +387,8 @@
                                $description->mType = 'Integer';
                        } elseif ( $fieldName == '_pageName' ) {
                                $description->mType = 'Page';
-                       } elseif ( strpos( $tableName, '(' ) !== false || 
strpos( $fieldName, '(' ) !== false ) {
-                               $sqlFunctionMatches = array();
-                               $sqlFunctionRegex = '/\b(\S*?)\s?\(/';
-                               preg_match_all( $sqlFunctionRegex, $fieldName, 
$sqlFunctionMatches );
-                               $sqlFunctions = array_map( 'strtoupper', 
$sqlFunctionMatches[1] );
-                               if ( count( $sqlFunctions ) == 0 ) {
-                                       // Must be in the "table name", then.
-                                       preg_match_all( $sqlFunctionRegex, 
$tableName, $sqlFunctionMatches );
-                                       $sqlFunctions = array_map( 
'strtoupper', $sqlFunctionMatches[1] );
-                               }
-
-                               // Throw an error if any of these functions
-                               // are not in our "whitelist" of SQL functions.
-                               foreach ( $sqlFunctions as $sqlFunction ) {
-                                       if ( !in_array( $sqlFunction, 
$wgCargoAllowedSQLFunctions ) ) {
-                                               throw new MWException( "Error: 
the SQL function \"$sqlFunction()\" is not allowed." );
-                                       }
-                               }
-
+                       } elseif ( strpos( $origFieldName, '(' ) !== false ) {
+                               $sqlFunctions = 
self::getAndValidateSQLFunctions( $origFieldName );
                                $firstFunction = $sqlFunctions[0];
                                if ( in_array( $firstFunction, array( 'COUNT', 
'FLOOR', 'CEIL', 'ROUND' ) ) ) {
                                        $description->mType = 'Integer';

-- 
To view, visit https://gerrit.wikimedia.org/r/227241
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I1e4ac5a712e1cc55d56585615cb9ac3545b51122
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Cargo
Gerrit-Branch: master
Gerrit-Owner: Yaron Koren <yaro...@gmail.com>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to