From da469e3abe8a5824e02605e0393dcf4f2354490a Mon Sep 17 00:00:00 2001 From: Daniel Friesen Date: Sat, 4 Dec 2010 09:27:02 +0000 Subject: [PATCH] Fix a bug in the installer caused by r77487 creating installer sql statements like "GRANT ALL PRIVILEGES ON `'dbname'`.* TO ''tablename''@'%" while improving the database independence of replaceVars. * Drop unused and likely broken /*$var*/` -> `$var syntax * Replace {$var} with '{$var}' and `{$var}` handling that uses relevant database independent quoting ({$var} without surrouding quotes are never used) * Give the generic/mysql class a proper quote_ident implementation * Fix the unused Oracle and Sqlite quote_ident implementations which are potential sql injections if used * Split common variable replacemnt code off to a replaceGlobalVars and make the generic and oracle code use it instead of duplicating the same code as each other --- includes/db/Database.php | 45 +++++++++++++++++++++++++------- includes/db/DatabaseOracle.php | 12 ++------- includes/db/DatabasePostgres.php | 8 +++--- includes/db/DatabaseSqlite.php | 2 +- 4 files changed, 43 insertions(+), 24 deletions(-) diff --git a/includes/db/Database.php b/includes/db/Database.php index b20f461c3d..e74d06efa6 100644 --- a/includes/db/Database.php +++ b/includes/db/Database.php @@ -1694,6 +1694,15 @@ abstract class DatabaseBase implements DatabaseType { } } + /** + * Quotes a string using `backticks` for things like database, table, and field + * names, other databases which use something other than backticks can replace + * this with something else + */ + function quote_ident( $s ) { + return "`" . $this->strencode( $s ) . "`"; + } + /** * Escape string for safe LIKE usage. * WARNING: you should almost never use this function directly, @@ -2500,6 +2509,32 @@ abstract class DatabaseBase implements DatabaseType { return true; } + /** + * Database independent variable replacement, replaces a set of named variables + * in a sql statement with the contents of their global variables. + * Supports '{$var}' `{$var}` and / *$var* / (without the spaces) style variables + * + * '{$var}' should be used for text and is passed through the database's addQuotes method + * `{$var}` should be used for identifiers (eg: table and database names), it is passed through + * the database's quote_ident method which can be overridden if the database + * uses something other than backticks. + * / *$var* / is just encoded, besides traditional dbprefix and tableoptions it's use should be avoided + * + * @param $ins String: SQL statement to replace variables in + * @param $varnames Array: Array of global variable names to replace + * @return String The new SQL statement with variables replaced + */ + protected function replaceGlobalVars( $ins, $varnames ) { + foreach ( $varnames as $var ) { + if ( isset( $GLOBALS[$var] ) ) { + $ins = str_replace( '\'{$' . $var . '}\'', $this->addQuotes( $GLOBALS[$var] ), $ins ); // replace '{$var}' + $ins = str_replace( '`{$' . $var . '}`', $this->quote_ident( $GLOBALS[$var] ), $ins ); // replace `{$var}` + $ins = str_replace( '/*$' . $var . '*/', $this->strencode( $GLOBALS[$var] ) , $ins ); // replace /*$var*/ + } + } + return $ins; + } + /** * Replace variables in sourced SQL */ @@ -2510,15 +2545,7 @@ abstract class DatabaseBase implements DatabaseType { 'wgDBadminuser', 'wgDBadminpassword', 'wgDBTableOptions', ); - // Ordinary variables - foreach ( $varnames as $var ) { - if ( isset( $GLOBALS[$var] ) ) { - $val = $this->addQuotes( $GLOBALS[$var] ); // FIXME: safety check? - $ins = str_replace( '{$' . $var . '}', $val, $ins ); - $ins = str_replace( '/*$' . $var . '*/`', '`' . $val, $ins ); - $ins = str_replace( '/*$' . $var . '*/', $val, $ins ); - } - } + $ins = $this->replaceGlobalVars( $ins, $varnames ); // Table prefixes $ins = preg_replace_callback( '!/\*(?:\$wgDBprefix|_)\*/([a-zA-Z_0-9]*)!', diff --git a/includes/db/DatabaseOracle.php b/includes/db/DatabaseOracle.php index 42b798e168..876188d123 100644 --- a/includes/db/DatabaseOracle.php +++ b/includes/db/DatabaseOracle.php @@ -1126,7 +1126,7 @@ class DatabaseOracle extends DatabaseBase { } function quote_ident( $s ) { - return $s; + return '"' . str_replace( '"', '""', $s ) . '"'; } function selectRow( $table, $vars, $conds, $fname = 'DatabaseOracle::selectRow', $options = array(), $join_conds = array() ) { @@ -1345,15 +1345,7 @@ class DatabaseOracle extends DatabaseBase { $varnames[] = '_OracleTempTS'; } - // Ordinary variables - foreach ( $varnames as $var ) { - if ( isset( $GLOBALS[$var] ) ) { - $val = $this->addQuotes( $GLOBALS[$var] ); // FIXME: safety check? - $ins = str_replace( '{$' . $var . '}', $val, $ins ); - $ins = str_replace( '/*$' . $var . '*/`', '`' . $val, $ins ); - $ins = str_replace( '/*$' . $var . '*/', $val, $ins ); - } - } + $ins = $this->replaceGlobalVars( $ins, $varnames ); return parent::replaceVars( $ins ); } diff --git a/includes/db/DatabasePostgres.php b/includes/db/DatabasePostgres.php index e651e94b16..ea768e28df 100644 --- a/includes/db/DatabasePostgres.php +++ b/includes/db/DatabasePostgres.php @@ -973,7 +973,7 @@ class DatabasePostgres extends DatabaseBase { * Return the next in a sequence, save the value for retrieval via insertId() */ function nextSequenceValue( $seqName ) { - $safeseq = preg_replace( "/'/", "''", $seqName ); + $safeseq = str_replace( "'", "''", $seqName ); $res = $this->query( "SELECT nextval('$safeseq')" ); $row = $this->fetchRow( $res ); $this->mInsertId = $row[0]; @@ -984,7 +984,7 @@ class DatabasePostgres extends DatabaseBase { * Return the current value of a sequence. Assumes it has been nextval'ed in this session. */ function currentSequenceValue( $seqName ) { - $safeseq = preg_replace( "/'/", "''", $seqName ); + $safeseq = str_replace( "'", "''", $seqName ); $res = $this->query( "SELECT currval('$safeseq')" ); $row = $this->fetchRow( $res ); $currval = $row[0]; @@ -1242,7 +1242,7 @@ SQL; * Query whether a given schema exists. Returns the name of the owner */ function schemaExists( $schema ) { - $eschema = preg_replace( "/'/", "''", $schema ); + $eschema = str_replace( "'", "''", $schema ); $SQL = "SELECT rolname FROM pg_catalog.pg_namespace n, pg_catalog.pg_roles r " ."WHERE n.nspowner=r.oid AND n.nspname = '$eschema'"; $res = $this->query( $SQL ); @@ -1301,7 +1301,7 @@ SQL; } function quote_ident( $s ) { - return '"' . preg_replace( '/"/', '""', $s ) . '"'; + return '"' . str_replace( '"', '""', $s ) . '"'; } /** diff --git a/includes/db/DatabaseSqlite.php b/includes/db/DatabaseSqlite.php index e29e1a48ff..5bc36cef8c 100644 --- a/includes/db/DatabaseSqlite.php +++ b/includes/db/DatabaseSqlite.php @@ -531,7 +531,7 @@ class DatabaseSqlite extends DatabaseBase { } function quote_ident( $s ) { - return $s; + return '"' . str_replace( '"', '""', $s ) . '"'; } function buildLike() { -- 2.20.1