Skip to content

Commit 7354773

Browse files
kamil-tekielakamil-tekiela
authored
Replace escapeString with quoteString (#17946)
* Replace escapeString with quoteString Signed-off-by: Kamil Tekiela <tekiela246@gmail.com> * Fix SQL injection Signed-off-by: Kamil Tekiela <tekiela246@gmail.com> Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
1 parent 93f9d89 commit 7354773

13 files changed

Lines changed: 93 additions & 112 deletions

File tree

‎libraries/classes/CreateAddField.php‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -398,8 +398,12 @@ public function getTableCreationQuery(string $db, string $table): string
398398
. Util::backquote(trim($table)) . ' (' . $sqlStatement . ')';
399399

400400
// Adds table type, character set, comments and partition definition
401-
if (! empty($_POST['tbl_storage_engine']) && ($_POST['tbl_storage_engine'] !== 'Default')) {
402-
$sqlQuery .= ' ENGINE = ' . $this->dbi->escapeString($_POST['tbl_storage_engine']);
401+
if (
402+
! empty($_POST['tbl_storage_engine'])
403+
&& ($_POST['tbl_storage_engine'] !== 'Default')
404+
&& StorageEngine::isValid($_POST['tbl_storage_engine'])
405+
) {
406+
$sqlQuery .= ' ENGINE = ' . $_POST['tbl_storage_engine'];
403407
}
404408

405409
if (! empty($_POST['tbl_collation'])) {
@@ -411,13 +415,11 @@ public function getTableCreationQuery(string $db, string $table): string
411415
&& ! empty($_POST['tbl_storage_engine'])
412416
&& $_POST['tbl_storage_engine'] === 'FEDERATED'
413417
) {
414-
$sqlQuery .= " CONNECTION = '"
415-
. $this->dbi->escapeString($_POST['connection']) . "'";
418+
$sqlQuery .= ' CONNECTION = ' . $this->dbi->quoteString($_POST['connection']);
416419
}
417420

418421
if (! empty($_POST['comment'])) {
419-
$sqlQuery .= ' COMMENT = \''
420-
. $this->dbi->escapeString($_POST['comment']) . '\'';
422+
$sqlQuery .= ' COMMENT = ' . $this->dbi->quoteString($_POST['comment']);
421423
}
422424

423425
$sqlQuery .= $this->getPartitionsDefinition();

‎libraries/classes/Partitioning/Partition.php‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -147,8 +147,8 @@ public static function getPartitions($db, $table)
147147
if (self::havePartitioning()) {
148148
$result = $GLOBALS['dbi']->fetchResult(
149149
'SELECT * FROM `information_schema`.`PARTITIONS`'
150-
. " WHERE `TABLE_SCHEMA` = '" . $GLOBALS['dbi']->escapeString($db)
151-
. "' AND `TABLE_NAME` = '" . $GLOBALS['dbi']->escapeString($table) . "'"
150+
. ' WHERE `TABLE_SCHEMA` = ' . $GLOBALS['dbi']->quoteString($db)
151+
. ' AND `TABLE_NAME` = ' . $GLOBALS['dbi']->quoteString($table)
152152
);
153153
if ($result) {
154154
$partitionMap = [];
@@ -192,8 +192,8 @@ public static function getPartitionNames($db, $table)
192192
if (self::havePartitioning()) {
193193
return $GLOBALS['dbi']->fetchResult(
194194
'SELECT DISTINCT `PARTITION_NAME` FROM `information_schema`.`PARTITIONS`'
195-
. " WHERE `TABLE_SCHEMA` = '" . $GLOBALS['dbi']->escapeString($db)
196-
. "' AND `TABLE_NAME` = '" . $GLOBALS['dbi']->escapeString($table) . "'"
195+
. ' WHERE `TABLE_SCHEMA` = ' . $GLOBALS['dbi']->quoteString($db)
196+
. ' AND `TABLE_NAME` = ' . $GLOBALS['dbi']->quoteString($table)
197197
);
198198
}
199199

@@ -213,8 +213,8 @@ public static function getPartitionMethod($db, $table)
213213
if (self::havePartitioning()) {
214214
$partition_method = $GLOBALS['dbi']->fetchResult(
215215
'SELECT `PARTITION_METHOD` FROM `information_schema`.`PARTITIONS`'
216-
. " WHERE `TABLE_SCHEMA` = '" . $GLOBALS['dbi']->escapeString($db) . "'"
217-
. " AND `TABLE_NAME` = '" . $GLOBALS['dbi']->escapeString($table) . "'"
216+
. ' WHERE `TABLE_SCHEMA` = ' . $GLOBALS['dbi']->quoteString($db)
217+
. ' AND `TABLE_NAME` = ' . $GLOBALS['dbi']->quoteString($table)
218218
. ' LIMIT 1'
219219
);
220220
if (! empty($partition_method)) {

‎libraries/classes/Plugins/Export/ExportSql.php‎

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -981,8 +981,7 @@ public function exportEvents($db): bool
981981

982982
$eventNames = $GLOBALS['dbi']->fetchResult(
983983
'SELECT EVENT_NAME FROM information_schema.EVENTS WHERE'
984-
. " EVENT_SCHEMA= '" . $GLOBALS['dbi']->escapeString($db)
985-
. "';"
984+
. ' EVENT_SCHEMA= ' . $GLOBALS['dbi']->quoteString($db)
986985
);
987986

988987
if ($eventNames) {
@@ -1141,7 +1140,7 @@ private function exportConfigurationMetadata(
11411140
$sqlQuery = 'SELECT `page_nr`, `page_descr` FROM '
11421141
. Util::backquote($relationParameters->pdfFeature->database)
11431142
. '.' . Util::backquote($relationParameters->pdfFeature->pdfPages)
1144-
. ' WHERE `db_name` = \'' . $GLOBALS['dbi']->escapeString($db) . "'";
1143+
. ' WHERE `db_name` = ' . $GLOBALS['dbi']->quoteString($db);
11451144

11461145
$result = $GLOBALS['dbi']->fetchResult($sqlQuery, 'page_nr', 'page_descr');
11471146

@@ -1150,8 +1149,8 @@ private function exportConfigurationMetadata(
11501149
$sqlQueryRow = 'SELECT `db_name`, `page_descr` FROM '
11511150
. Util::backquote($relationParameters->pdfFeature->database)
11521151
. '.' . Util::backquote($relationParameters->pdfFeature->pdfPages)
1153-
. ' WHERE `db_name` = \'' . $GLOBALS['dbi']->escapeString($db) . "'"
1154-
. " AND `page_nr` = '" . intval($page) . "'";
1152+
. ' WHERE `db_name` = ' . $GLOBALS['dbi']->quoteString($db)
1153+
. ' AND `page_nr` = ' . intval($page);
11551154

11561155
if (
11571156
! $this->exportData(
@@ -1216,10 +1215,9 @@ private function exportConfigurationMetadata(
12161215
$sqlQuery .= Util::backquote($relationParameters->db)
12171216
. '.' . Util::backquote((string) $relationParams[$type])
12181217
. ' WHERE ' . Util::backquote($dbNameColumn)
1219-
. " = '" . $GLOBALS['dbi']->escapeString($db) . "'";
1218+
. ' = ' . $GLOBALS['dbi']->quoteString($db);
12201219
if (isset($table)) {
1221-
$sqlQuery .= " AND `table_name` = '"
1222-
. $GLOBALS['dbi']->escapeString($table) . "'";
1220+
$sqlQuery .= ' AND `table_name` = ' . $GLOBALS['dbi']->quoteString($table);
12231221
}
12241222

12251223
if (
@@ -1330,17 +1328,15 @@ private function getTableDefForView(
13301328
}
13311329

13321330
if (isset($column['Default'])) {
1333-
$createQuery .= " DEFAULT '"
1334-
. $GLOBALS['dbi']->escapeString($column['Default']) . "'";
1331+
$createQuery .= ' DEFAULT ' . $GLOBALS['dbi']->quoteString($column['Default']);
13351332
} else {
13361333
if ($column['Null'] === 'YES') {
13371334
$createQuery .= ' DEFAULT NULL';
13381335
}
13391336
}
13401337

13411338
if (! empty($column['Comment'])) {
1342-
$createQuery .= " COMMENT '"
1343-
. $GLOBALS['dbi']->escapeString($column['Comment']) . "'";
1339+
$createQuery .= ' COMMENT ' . $GLOBALS['dbi']->quoteString($column['Comment']);
13441340
}
13451341

13461342
$firstCol = false;
@@ -1405,7 +1401,7 @@ public function getTableDef(
14051401

14061402
$result = $GLOBALS['dbi']->tryQuery(
14071403
'SHOW TABLE STATUS FROM ' . Util::backquote($db)
1408-
. ' WHERE Name = \'' . $GLOBALS['dbi']->escapeString((string) $table) . '\''
1404+
. ' WHERE Name = ' . $GLOBALS['dbi']->quoteString((string) $table)
14091405
);
14101406
if ($result != false) {
14111407
if ($result->numRows() > 0) {
@@ -2362,7 +2358,7 @@ public function exportData(
23622358
$values[] = "''";
23632359
} else {
23642360
// something else -> treat as a string
2365-
$values[] = '\'' . $GLOBALS['dbi']->escapeString($row[$j]) . '\'';
2361+
$values[] = $GLOBALS['dbi']->quoteString($row[$j]);
23662362
}
23672363
}
23682364

‎libraries/classes/Plugins/Export/ExportXml.php‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,7 @@ public function exportHeader(): bool
238238
$result = $GLOBALS['dbi']->fetchResult(
239239
'SELECT `DEFAULT_CHARACTER_SET_NAME`, `DEFAULT_COLLATION_NAME`'
240240
. ' FROM `information_schema`.`SCHEMATA` WHERE `SCHEMA_NAME`'
241-
. ' = \'' . $GLOBALS['dbi']->escapeString($GLOBALS['db']) . '\' LIMIT 1'
241+
. ' = ' . $GLOBALS['dbi']->quoteString($GLOBALS['db']) . ' LIMIT 1'
242242
);
243243
$db_collation = $result[0]['DEFAULT_COLLATION_NAME'];
244244
$db_charset = $result[0]['DEFAULT_CHARACTER_SET_NAME'];
@@ -337,8 +337,7 @@ public function exportHeader(): bool
337337
// Export events
338338
$events = $GLOBALS['dbi']->fetchResult(
339339
'SELECT EVENT_NAME FROM information_schema.EVENTS '
340-
. "WHERE EVENT_SCHEMA='" . $GLOBALS['dbi']->escapeString($GLOBALS['db'])
341-
. "'"
340+
. 'WHERE EVENT_SCHEMA=' . $GLOBALS['dbi']->quoteString($GLOBALS['db'])
342341
);
343342
$head .= $this->exportDefinitions($GLOBALS['db'], 'event', $events);
344343
}

‎libraries/classes/Plugins/Import/ImportCsv.php‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -522,9 +522,7 @@ public function doImport(?File $importHandle = null): array
522522
if ($val === null) {
523523
$sql .= 'NULL';
524524
} else {
525-
$sql .= '\''
526-
. $GLOBALS['dbi']->escapeString($val)
527-
. '\'';
525+
$sql .= $GLOBALS['dbi']->quoteString($val);
528526
}
529527

530528
$first = false;

‎libraries/classes/Plugins/Import/ImportLdi.php‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -125,8 +125,7 @@ public function doImport(?File $importHandle = null): array
125125
$sql .= ' LOCAL';
126126
}
127127

128-
$sql .= ' INFILE \'' . $GLOBALS['dbi']->escapeString($GLOBALS['import_file'])
129-
. '\'';
128+
$sql .= ' INFILE ' . $GLOBALS['dbi']->quoteString($GLOBALS['import_file']);
130129
if (isset($GLOBALS['ldi_replace'])) {
131130
$sql .= ' REPLACE';
132131
} elseif (isset($GLOBALS['ldi_ignore'])) {
@@ -140,13 +139,11 @@ public function doImport(?File $importHandle = null): array
140139
}
141140

142141
if (strlen((string) $GLOBALS['ldi_enclosed']) > 0) {
143-
$sql .= ' ENCLOSED BY \''
144-
. $GLOBALS['dbi']->escapeString($GLOBALS['ldi_enclosed']) . '\'';
142+
$sql .= ' ENCLOSED BY ' . $GLOBALS['dbi']->quoteString($GLOBALS['ldi_enclosed']);
145143
}
146144

147145
if (strlen((string) $GLOBALS['ldi_escaped']) > 0) {
148-
$sql .= ' ESCAPED BY \''
149-
. $GLOBALS['dbi']->escapeString($GLOBALS['ldi_escaped']) . '\'';
146+
$sql .= ' ESCAPED BY ' . $GLOBALS['dbi']->quoteString($GLOBALS['ldi_escaped']);
150147
}
151148

152149
if (strlen((string) $GLOBALS['ldi_new_line']) > 0) {

‎libraries/classes/Plugins/Schema/Pdf/Pdf.php‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
namespace PhpMyAdmin\Plugins\Schema\Pdf;
99

1010
use PhpMyAdmin\ConfigStorage\Relation;
11+
use PhpMyAdmin\DatabaseInterface;
1112
use PhpMyAdmin\Pdf as PdfLib;
1213
use PhpMyAdmin\Util;
1314

@@ -271,8 +272,8 @@ public function Header(): void
271272
$test_query = 'SELECT * FROM '
272273
. Util::backquote($pdfFeature->database) . '.'
273274
. Util::backquote($pdfFeature->pdfPages)
274-
. ' WHERE db_name = \'' . $GLOBALS['dbi']->escapeString($this->db)
275-
. '\' AND page_nr = \'' . $this->pageNumber . '\'';
275+
. ' WHERE db_name = ' . $GLOBALS['dbi']->quoteString($this->db, DatabaseInterface::CONNECT_CONTROL)
276+
. ' AND page_nr = ' . $this->pageNumber;
276277
$test_rs = $GLOBALS['dbi']->queryAsControlUser($test_query);
277278
$pageDesc = (string) $test_rs->fetchValue('page_descr');
278279

‎libraries/classes/Server/Status/Monitor.php‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
use function array_sum;
1616
use function count;
1717
use function implode;
18-
use function is_numeric;
1918
use function json_decode;
2019
use function mb_strlen;
2120
use function mb_strpos;
@@ -480,13 +479,8 @@ private function getSuspensionPoints(string $lastChar): string
480479
public function getJsonForLoggingVars(?string $name, ?string $value): array
481480
{
482481
if (isset($name, $value)) {
483-
$escapedValue = $this->dbi->escapeString($value);
484-
if (! is_numeric($escapedValue)) {
485-
$escapedValue = "'" . $escapedValue . "'";
486-
}
487-
488482
if (! preg_match('/[^a-zA-Z0-9_]+/', $name)) {
489-
$this->dbi->query('SET GLOBAL ' . $name . ' = ' . $escapedValue);
483+
$this->dbi->query('SET GLOBAL ' . $name . ' = ' . $this->dbi->quoteString($value));
490484
}
491485
}
492486

‎psalm-baseline.xml‎

Lines changed: 4 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -4353,17 +4353,13 @@
43534353
</RedundantCondition>
43544354
</file>
43554355
<file src="libraries/classes/CreateAddField.php">
4356-
<DeprecatedMethod occurrences="6">
4357-
<code>escapeString</code>
4358-
<code>escapeString</code>
4359-
<code>escapeString</code>
4356+
<DeprecatedMethod occurrences="3">
43604357
<code>escapeString</code>
43614358
<code>escapeString</code>
43624359
<code>escapeString</code>
43634360
</DeprecatedMethod>
43644361
<LessSpecificReturnStatement occurrences="1"/>
4365-
<MixedArgument occurrences="13">
4366-
<code>$_POST['comment']</code>
4362+
<MixedArgument occurrences="12">
43674363
<code>$_POST['field_name'][$column['col_index']]</code>
43684364
<code>$fieldPrimary[0]</code>
43694365
<code>$index</code>
@@ -4477,13 +4473,14 @@
44774473
<PossiblyInvalidIterator occurrences="1">
44784474
<code>$_POST['partitions']</code>
44794475
</PossiblyInvalidIterator>
4480-
<PossiblyInvalidOperand occurrences="6">
4476+
<PossiblyInvalidOperand occurrences="7">
44814477
<code>$_POST['partition_by']</code>
44824478
<code>$_POST['partition_count']</code>
44834479
<code>$_POST['partition_expr']</code>
44844480
<code>$_POST['subpartition_by']</code>
44854481
<code>$_POST['subpartition_count']</code>
44864482
<code>$_POST['subpartition_expr']</code>
4483+
<code>$_POST['tbl_storage_engine']</code>
44874484
</PossiblyInvalidOperand>
44884485
</file>
44894486
<file src="libraries/classes/Crypto/Crypto.php">
@@ -8965,14 +8962,6 @@
89658962
</MixedAssignment>
89668963
</file>
89678964
<file src="libraries/classes/Partitioning/Partition.php">
8968-
<DeprecatedMethod occurrences="6">
8969-
<code>escapeString</code>
8970-
<code>escapeString</code>
8971-
<code>escapeString</code>
8972-
<code>escapeString</code>
8973-
<code>escapeString</code>
8974-
<code>escapeString</code>
8975-
</DeprecatedMethod>
89768965
<MixedArrayAccess occurrences="1">
89778966
<code>$value['Name']</code>
89788967
</MixedArrayAccess>
@@ -9660,17 +9649,6 @@
96609649
</PossiblyNullArgument>
96619650
</file>
96629651
<file src="libraries/classes/Plugins/Export/ExportSql.php">
9663-
<DeprecatedMethod occurrences="9">
9664-
<code>escapeString</code>
9665-
<code>escapeString</code>
9666-
<code>escapeString</code>
9667-
<code>escapeString</code>
9668-
<code>escapeString</code>
9669-
<code>escapeString</code>
9670-
<code>escapeString</code>
9671-
<code>escapeString</code>
9672-
<code>escapeString</code>
9673-
</DeprecatedMethod>
96749652
<InvalidArgument occurrences="2">
96759653
<code>$GLOBALS['asfile']</code>
96769654
<code>$GLOBALS['sql_if_not_exists']</code>
@@ -9949,10 +9927,6 @@
99499927
</PossiblyUndefinedVariable>
99509928
</file>
99519929
<file src="libraries/classes/Plugins/Export/ExportXml.php">
9952-
<DeprecatedMethod occurrences="2">
9953-
<code>escapeString</code>
9954-
<code>escapeString</code>
9955-
</DeprecatedMethod>
99569930
<MixedArgument occurrences="7">
99579931
<code>$code</code>
99589932
<code>$col_as</code>
@@ -10352,9 +10326,6 @@
1035210326
</MixedReturnStatement>
1035310327
</file>
1035410328
<file src="libraries/classes/Plugins/Import/ImportCsv.php">
10355-
<DeprecatedMethod occurrences="1">
10356-
<code>escapeString</code>
10357-
</DeprecatedMethod>
1035810329
<DocblockTypeContradiction occurrences="2">
1035910330
<code>$nameArray === false</code>
1036010331
<code>$nameArray === false</code>
@@ -10447,11 +10418,6 @@
1044710418
</RedundantCast>
1044810419
</file>
1044910420
<file src="libraries/classes/Plugins/Import/ImportLdi.php">
10450-
<DeprecatedMethod occurrences="3">
10451-
<code>escapeString</code>
10452-
<code>escapeString</code>
10453-
<code>escapeString</code>
10454-
</DeprecatedMethod>
1045510421
<MixedArgument occurrences="3">
1045610422
<code>$GLOBALS['ldi_columns']</code>
1045710423
<code>$GLOBALS['ldi_enclosed']</code>
@@ -11033,9 +10999,6 @@
1103310999
</RiskyCast>
1103411000
</file>
1103511001
<file src="libraries/classes/Plugins/Schema/Pdf/Pdf.php">
11036-
<DeprecatedMethod occurrences="1">
11037-
<code>escapeString</code>
11038-
</DeprecatedMethod>
1103911002
<MixedArgument occurrences="12">
1104011003
<code>$data[$i]</code>
1104111004
<code>$data[$i]</code>
@@ -12743,9 +12706,6 @@
1274312706
</RedundantCast>
1274412707
</file>
1274512708
<file src="libraries/classes/Server/Status/Monitor.php">
12746-
<DeprecatedMethod occurrences="1">
12747-
<code>escapeString</code>
12748-
</DeprecatedMethod>
1274912709
<InvalidArrayAccess occurrences="1">
1275012710
<code>$temp[strlen($temp) - 1]</code>
1275112711
</InvalidArrayAccess>

‎test/classes/CreateAddFieldTest.php‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -264,7 +264,24 @@ public function providerGetTableCreationQuery(): array
264264
],
265265
],
266266
[
267-
'CREATE TABLE `db`.`table` () ENGINE = Inno\\\'DB CHARSET=armscii8 COMMENT = \'my \\\'table\';',
267+
'CREATE TABLE `db`.`table` () ENGINE = dummy CHARSET=armscii8 COMMENT = \'my \\\'table\';',
268+
'db',
269+
'table',
270+
[
271+
'field_name' => [],
272+
'primary_indexes' => '{}',
273+
'indexes' => '{}',
274+
'unique_indexes' => '{}',
275+
'fulltext_indexes' => '{}',
276+
'spatial_indexes' => '{}',
277+
'tbl_storage_engine' => 'dummy',
278+
'tbl_collation' => 'armscii8',
279+
'connection' => 'aaaa',
280+
'comment' => 'my \'table',
281+
],
282+
],
283+
[
284+
'CREATE TABLE `db`.`table` () CHARSET=armscii8 COMMENT = \'my \\\'table\';',
268285
'db',
269286
'table',
270287
[

0 commit comments

Comments
 (0)