From 6d0a30a391cfb78cb51a476b4f925a625070c4b6 Mon Sep 17 00:00:00 2001 From: Kamil Tekiela Date: Sat, 4 Jun 2022 19:06:16 +0100 Subject: [PATCH 1/2] Fix errors in InsertEdit Signed-off-by: Kamil Tekiela --- .../Controllers/Table/ReplaceController.php | 56 ++++++++--------- libraries/classes/InsertEdit.php | 26 ++------ phpstan-baseline.neon | 5 ++ psalm-baseline.xml | 9 ++- test/classes/InsertEditTest.php | 60 +++++++++---------- 5 files changed, 76 insertions(+), 80 deletions(-) diff --git a/libraries/classes/Controllers/Table/ReplaceController.php b/libraries/classes/Controllers/Table/ReplaceController.php index 6604494085..a841b1c333 100644 --- a/libraries/classes/Controllers/Table/ReplaceController.php +++ b/libraries/classes/Controllers/Table/ReplaceController.php @@ -308,33 +308,35 @@ final class ReplaceController extends AbstractController // delete $file_to_insert temporary variable $file_to_insert->cleanUp(); - $current_value = $this->insertEdit->getCurrentValueForDifferentTypes( - $possibly_uploaded_val, - $key, - $multi_edit_columns_type, - $current_value, - $multi_edit_auto_increment, - $rownumber, - $multi_edit_columns_name, - $multi_edit_columns_null, - $multi_edit_columns_null_prev, - $is_insert, - $using_key, - $where_clause, - $table, - $multi_edit_funcs - ); - - $current_value_as_an_array = $this->insertEdit->getCurrentValueAsAnArrayForMultipleEdit( - $multi_edit_funcs, - $multi_edit_salt, - $gis_from_text_functions, - $current_value, - $gis_from_wkb_functions, - $func_optional_param, - $func_no_param, - $key - ); + if (empty($multi_edit_funcs[$key])) { + $current_value_as_an_array = $this->insertEdit->getCurrentValueForDifferentTypes( + $possibly_uploaded_val, + $key, + $multi_edit_columns_type, + $current_value, + $multi_edit_auto_increment, + $rownumber, + $multi_edit_columns_name, + $multi_edit_columns_null, + $multi_edit_columns_null_prev, + $is_insert, + $using_key, + $where_clause, + $table, + $multi_edit_funcs + ); + } else { + $current_value_as_an_array = $this->insertEdit->getCurrentValueAsAnArrayForMultipleEdit( + $multi_edit_funcs, + $multi_edit_salt, + $gis_from_text_functions, + $current_value, + $gis_from_wkb_functions, + $func_optional_param, + $func_no_param, + $key + ); + } if (! isset($multi_edit_virtual, $multi_edit_virtual[$key])) { [ diff --git a/libraries/classes/InsertEdit.php b/libraries/classes/InsertEdit.php index 5976d8b612..7ea09e576f 100644 --- a/libraries/classes/InsertEdit.php +++ b/libraries/classes/InsertEdit.php @@ -35,7 +35,6 @@ use function max; use function mb_stripos; use function mb_strlen; use function mb_strstr; -use function mb_substr; use function md5; use function method_exists; use function min; @@ -1572,10 +1571,6 @@ class InsertEdit $funcNoParam, $key ): string { - if (empty($multiEditFuncs[$key])) { - return $currentValue; - } - if ($multiEditFuncs[$key] === 'PHP_PASSWORD_HASH') { /** * @see https://github.com/vimeo/psalm/issues/3350 @@ -1584,28 +1579,21 @@ class InsertEdit */ $hash = password_hash($currentValue, PASSWORD_DEFAULT); - return "'" . $hash . "'"; + return "'" . $this->dbi->escapeString($hash) . "'"; } if ($multiEditFuncs[$key] === 'UUID') { /* This way user will know what UUID new row has */ $uuid = (string) $this->dbi->fetchValue('SELECT UUID()'); - return "'" . $uuid . "'"; + return "'" . $this->dbi->escapeString($uuid) . "'"; } if ( in_array($multiEditFuncs[$key], $gisFromTextFunctions) || in_array($multiEditFuncs[$key], $gisFromWkbFunctions) ) { - // Remove enclosing apostrophes - $currentValue = mb_substr($currentValue, 1, -1); - // Remove escaping apostrophes - $currentValue = str_replace("''", "'", $currentValue); - // Remove backslash-escaped apostrophes - $currentValue = str_replace("\'", "'", $currentValue); - - return $multiEditFuncs[$key] . '(' . $currentValue . ')'; + return $multiEditFuncs[$key] . "('" . $this->dbi->escapeString($currentValue) . "')"; } if ( @@ -1622,11 +1610,11 @@ class InsertEdit || $multiEditFuncs[$key] === 'DES_DECRYPT' || $multiEditFuncs[$key] === 'ENCRYPT')) ) { - return $multiEditFuncs[$key] . '(' . $currentValue . ",'" + return $multiEditFuncs[$key] . "('" . $this->dbi->escapeString($currentValue) . "','" . $this->dbi->escapeString($multiEditSalt[$key]) . "')"; } - return $multiEditFuncs[$key] . '(' . $currentValue . ')'; + return $multiEditFuncs[$key] . "('" . $this->dbi->escapeString($currentValue) . "')"; } return $multiEditFuncs[$key] . '()'; @@ -1745,10 +1733,6 @@ class InsertEdit return $possiblyUploadedVal; } - if (! empty($multiEditFuncs[$key])) { - return "'" . $this->dbi->escapeString($currentValue) . "'"; - } - // c o l u m n v a l u e i n t h e f o r m $type = $multiEditColumnsType[$key] ?? ''; diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 807538fa29..bea8d78d03 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -5040,6 +5040,11 @@ parameters: count: 1 path: libraries/classes/InsertEdit.php + - + message: "#^Parameter \\#1 \\$str of method PhpMyAdmin\\\\DatabaseInterface\\:\\:escapeString\\(\\) expects string, string\\|false given\\.$#" + count: 1 + path: libraries/classes/InsertEdit.php + - message: "#^Parameter \\#1 \\$value of static method PhpMyAdmin\\\\Util\\:\\:addMicroseconds\\(\\) expects string, string\\|null given\\.$#" count: 1 diff --git a/psalm-baseline.xml b/psalm-baseline.xml index be430c4b4b..d4c577bce4 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -3335,7 +3335,7 @@ $insert_errors - + $_POST['db'] $_POST['rel_fields_list'] $_POST['table'] @@ -3345,6 +3345,8 @@ $column_name $current_value $current_value + $current_value + $current_value $db $db $db @@ -3449,10 +3451,13 @@ $where_clause $where_clause - + + $current_value $current_value $current_value $multi_edit_columns_null + $multi_edit_columns_null + $multi_edit_columns_null_prev $multi_edit_columns_null_prev $multi_edit_columns_prev $multi_edit_funcs diff --git a/test/classes/InsertEditTest.php b/test/classes/InsertEditTest.php index 539a91eb60..1c7ad8eee3 100644 --- a/test/classes/InsertEditTest.php +++ b/test/classes/InsertEditTest.php @@ -17,7 +17,9 @@ use ReflectionProperty; use stdClass; use function hash; +use function mb_substr; use function md5; +use function password_verify; use function sprintf; use const MYSQLI_PRI_KEY_FLAG; @@ -2028,32 +2030,15 @@ class InsertEditTest extends AbstractTestCase */ public function testGetCurrentValueAsAnArrayForMultipleEdit(): void { - $result = $this->insertEdit->getCurrentValueAsAnArrayForMultipleEdit( - [], - [], - [], - 'currVal', - [], - [], - [], - '0' - ); - - $this->assertEquals('currVal', $result); - // case 2 $multi_edit_funcs = ['UUID']; - $dbi = $this->getMockBuilder(DatabaseInterface::class) - ->disableOriginalConstructor() - ->getMock(); - - $dbi->expects($this->once()) - ->method('fetchValue') - ->with('SELECT UUID()') - ->will($this->returnValue('uuid1234')); - - $GLOBALS['dbi'] = $dbi; + $this->dummyDbi->addResult( + 'SELECT UUID()', + [ + ['uuid1234'],// Minimal working setup for 2FA + ] + ); $this->insertEdit = new InsertEdit($GLOBALS['dbi']); $result = $this->insertEdit->getCurrentValueAsAnArrayForMultipleEdit( @@ -2076,13 +2061,13 @@ class InsertEditTest extends AbstractTestCase $multi_edit_funcs, $multi_edit_salt, [], - "'''", + "'", [], ['func'], ['func'], '0' ); - $this->assertEquals("AES_ENCRYPT(''','')", $result); + $this->assertEquals("AES_ENCRYPT('\\'','')", $result); // case 4 $multi_edit_funcs = ['func']; @@ -2091,26 +2076,41 @@ class InsertEditTest extends AbstractTestCase $multi_edit_funcs, $multi_edit_salt, [], - "'''", + "'", [], ['func'], ['func'], '0' ); - $this->assertEquals("func(''')", $result); + $this->assertEquals("func('\\'')", $result); // case 5 + $multi_edit_funcs = ['RAND']; $result = $this->insertEdit->getCurrentValueAsAnArrayForMultipleEdit( $multi_edit_funcs, $multi_edit_salt, [], - "''", + '', [], ['func'], - ['func'], + ['RAND'], '0' ); - $this->assertEquals('func()', $result); + $this->assertEquals('RAND()', $result); + + // case 6 + $multi_edit_funcs = ['PHP_PASSWORD_HASH']; + $result = $this->insertEdit->getCurrentValueAsAnArrayForMultipleEdit( + $multi_edit_funcs, + $multi_edit_salt, + [], + "a'c", + [], + [], + [], + '0' + ); + $this->assertTrue(password_verify("a'c", mb_substr($result, 1, -1))); } /** From d4942baecdd7363fb78e9eb394c79d3f7a0eed46 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maur=C3=ADcio=20Meneghini=20Fauth?= Date: Sat, 11 Jun 2022 11:17:04 -0300 Subject: [PATCH 2/2] Add ChangeLog entry for #17121 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit [ci skip] Signed-off-by: Maurício Meneghini Fauth --- ChangeLog | 1 + 1 file changed, 1 insertion(+) diff --git a/ChangeLog b/ChangeLog index e4f2816ae0..fd6a4178ff 100644 --- a/ChangeLog +++ b/ChangeLog @@ -13,6 +13,7 @@ phpMyAdmin - ChangeLog - issue #17424 Fix export limit size calculation - issue #17366 Fix refresh rate popup on Monitor page - issue #17577 Fix monitor charts size on RTL languages +- issue #17121 Fix password_hash function incorrectly adding single quotes to password before hashing 5.2.0 (2022-05-10) - issue #16521 Upgrade Bootstrap to version 5