From a059cdba47689782499132ee454e71d4480b7e89 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maur=C3=ADcio=20Meneghini=20Fauth?= Date: Tue, 20 Jul 2021 12:07:11 -0300 Subject: [PATCH 01/13] Add config directive to show/hide connection errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Maurício Meneghini Fauth --- doc/config.rst | 14 +++++++++ libraries/classes/Dbi/DbiMysqli.php | 46 ++++++++++++++++++++++------- libraries/config.default.php | 7 +++++ 3 files changed, 57 insertions(+), 10 deletions(-) diff --git a/doc/config.rst b/doc/config.rst index 183b9f5ce9..e9471f7faf 100644 --- a/doc/config.rst +++ b/doc/config.rst @@ -1477,6 +1477,20 @@ Server connection settings after logout (doesn't affect config authentication method). Should be absolute including protocol. +.. config:option:: $cfg['Servers'][$i]['hide_connection_errors'] + + :type: boolean + :default: false + + .. versionadded:: 4.9.8 + + Whether to show or hide detailed MySQL/MariaDB connection errors on the login page. + + .. note:: + + This error message can contain the target database server hostname or IP address, + which may reveal information about your network to an attacker. + Generic settings ---------------- diff --git a/libraries/classes/Dbi/DbiMysqli.php b/libraries/classes/Dbi/DbiMysqli.php index afd596ccd7..d0905eb00c 100644 --- a/libraries/classes/Dbi/DbiMysqli.php +++ b/libraries/classes/Dbi/DbiMysqli.php @@ -149,16 +149,29 @@ class DbiMysqli implements DbiExtension $host = $server['host']; } - $return_value = mysqli_real_connect( - $link, - $host, - $user, - $password, - '', - $server['port'], - $server['socket'], - $client_flags - ); + if ($server['hide_connection_errors']) { + $return_value = @mysqli_real_connect( + $link, + $host, + $user, + $password, + '', + $server['port'], + $server['socket'], + $client_flags + ); + } else { + $return_value = mysqli_real_connect( + $link, + $host, + $user, + $password, + '', + $server['port'], + $server['socket'], + $client_flags + ); + } if ($return_value === false || is_null($return_value)) { /* @@ -180,7 +193,20 @@ class DbiMysqli implements DbiExtension ); $server['ssl'] = true; return self::connect($user, $password, $server); + } elseif ($error_number === 1045 && $server['hide_connection_errors']) { + trigger_error( + sprintf( + __( + 'Error 1045: Access denied for user. Additional error information' + . ' may be available, but is being hidden by the %s configuration directive.' + ), + '[code][doc@cfg_Servers_hide_connection_errors]' + . '$cfg[\'Servers\'][$i][\'hide_connection_errors\'][/doc][/code]' + ), + E_USER_ERROR + ); } + return false; } diff --git a/libraries/config.default.php b/libraries/config.default.php index e7cf828bff..fcd54c9a18 100644 --- a/libraries/config.default.php +++ b/libraries/config.default.php @@ -603,6 +603,13 @@ $cfg['Servers'][$i]['tracking_add_drop_table'] = true; */ $cfg['Servers'][$i]['tracking_add_drop_database'] = true; +/** + * Whether to show or hide detailed MySQL/MariaDB connection errors on the login page. + * + * @global bool $cfg['Servers'][$i]['hide_connection_errors'] + */ +$cfg['Servers'][$i]['hide_connection_errors'] = false; + /** * Default server (0 = no default server) * From eb9bcbc040ace2a99375fb043c7de4f44533ea10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maur=C3=ADcio=20Meneghini=20Fauth?= Date: Wed, 13 Oct 2021 11:51:56 -0300 Subject: [PATCH 02/13] Encrypt the URL query when sensitive data is present MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Maurício Meneghini Fauth --- libraries/classes/Url.php | 53 +++++++++++++++++++++++++++++++++++++++ libraries/common.inc.php | 12 +++++++++ 2 files changed, 65 insertions(+) diff --git a/libraries/classes/Url.php b/libraries/classes/Url.php index f527819fbb..d63d7b6989 100644 --- a/libraries/classes/Url.php +++ b/libraries/classes/Url.php @@ -7,6 +7,9 @@ */ namespace PhpMyAdmin; +use phpseclib\Crypt\AES; +use phpseclib\Crypt\Random; + /** * Static methods for URL/hidden inputs generating * @@ -220,6 +223,11 @@ class Url $query = http_build_query($params, null, $separator); + if (isset($params['db'])) { + $encryptedQuery = self::encryptQuery($query); + $query = http_build_query(['eq' => $encryptedQuery], null, $separator); + } + if ($divider != '?' || strlen($query) > 0) { return $divider . $query; } @@ -227,6 +235,51 @@ class Url return ''; } + /** + * @param string $query + * @return string + */ + public static function encryptQuery($query) + { + global $PMA_Config; + + $key = $_SESSION[' HMAC_secret '] . $PMA_Config->get('blowfish_secret'); + $cipher = new AES(AES::MODE_CBC); + $iv = Random::string(16); + $cipher->setIV($iv); + $cipher->setKey($key); + $ciphertext = $cipher->encrypt($query); + $hmac = hash_hmac('sha256', $iv . $ciphertext, $key, true); + + return strtr(base64_encode($hmac . $iv . $ciphertext), '+/', '-_'); + } + + /** + * @param string $query + * @return string|null + */ + public static function decryptQuery($query) + { + global $PMA_Config; + + $encryptedQuery = base64_decode(strtr($query, '-_', '+/')); + $hmac = mb_substr($encryptedQuery, 0, 32, '8bit'); + $iv = mb_substr($encryptedQuery, 32, 16, '8bit'); + $ciphertext = mb_substr($encryptedQuery, 48, null, '8bit'); + $key = $_SESSION[' HMAC_secret '] . $PMA_Config->get('blowfish_secret'); + $calculated = hash_hmac('sha256', $iv . $ciphertext, $key, true); + + if (! hash_equals($hmac, $calculated)) { + return null; + } + + $cipher = new AES(AES::MODE_CBC); + $cipher->setIV($iv); + $cipher->setKey($key); + + return $cipher->decrypt($ciphertext); + } + /** * Returns url separator * diff --git a/libraries/common.inc.php b/libraries/common.inc.php index 8ec80a9638..fafc6d7353 100644 --- a/libraries/common.inc.php +++ b/libraries/common.inc.php @@ -43,6 +43,7 @@ use PhpMyAdmin\Response; use PhpMyAdmin\Session; use PhpMyAdmin\ThemeManager; use PhpMyAdmin\Tracker; +use PhpMyAdmin\Url; use PhpMyAdmin\Util; /** @@ -132,6 +133,17 @@ if (! defined('PMA_NO_SESSION')) { Session::setUp($GLOBALS['PMA_Config'], $GLOBALS['error_handler']); } +if (isset($_GET['eq']) && is_string($_GET['eq'])) { + $decryptedQuery = Url::decryptQuery($_GET['eq']); + if ($decryptedQuery !== null) { + parse_str($decryptedQuery, $urlQueryParams); + foreach ($urlQueryParams as $urlQueryParamKey => $urlQueryParamValue) { + $_GET[$urlQueryParamKey] = $urlQueryParamValue; + $_REQUEST[$urlQueryParamKey] = $urlQueryParamValue; + } + } +} + /** * init some variables LABEL_variables_init */ From 28ad63c96916ac1f81b7266faab65d3e7c9205ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maur=C3=ADcio=20Meneghini=20Fauth?= Date: Thu, 14 Oct 2021 12:01:12 -0300 Subject: [PATCH 03/13] Create `PhpMyAdmin\Crypto\Crypto` class to handle encryptions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Maurício Meneghini Fauth --- composer.json | 2 +- doc/config.rst | 11 +++ libraries/classes/Crypto/Crypto.php | 80 +++++++++++++++++++ libraries/classes/Url.php | 36 +-------- libraries/config.default.php | 7 ++ test/classes/Config/PageSettingsTest.php | 1 + .../Server/ServerDatabasesControllerTest.php | 1 + test/classes/Display/ResultsTest.php | 1 + test/classes/FooterTest.php | 1 + test/classes/InsertEditTest.php | 1 + .../classes/Navigation/NavigationTreeTest.php | 1 + .../Plugins/Auth/AuthenticationCookieTest.php | 1 + test/classes/PmaTestCase.php | 1 + 13 files changed, 111 insertions(+), 33 deletions(-) create mode 100644 libraries/classes/Crypto/Crypto.php diff --git a/composer.json b/composer.json index 2423a2565a..c9fcfd9453 100644 --- a/composer.json +++ b/composer.json @@ -65,7 +65,7 @@ "samyoul/u2f-php-server": "<1.1" }, "suggest": { - "ext-openssl": "Cookie encryption", + "ext-openssl": "For encryption performance", "ext-curl": "Updates checking", "ext-opcache": "Better performance", "ext-zlib": "For gz import and export", diff --git a/doc/config.rst b/doc/config.rst index 183b9f5ce9..f0b9ae2218 100644 --- a/doc/config.rst +++ b/doc/config.rst @@ -1736,6 +1736,17 @@ Generic settings Define whether phpMyAdmin will continue executing a multi-query statement if one of the queries fails. Default is to abort execution. +.. config:option:: $cfg['URLQueryEncryption'] + + :type: boolean + :default: true + + .. versionadded:: 4.9.8 + + Define whether phpMyAdmin will encrypt sensitive data (like database name + and table name) from the URL query string. Default is to encrypt the URL + query string. + Cookie authentication options ----------------------------- diff --git a/libraries/classes/Crypto/Crypto.php b/libraries/classes/Crypto/Crypto.php new file mode 100644 index 0000000000..128598639d --- /dev/null +++ b/libraries/classes/Crypto/Crypto.php @@ -0,0 +1,80 @@ +setIV($iv); + $cipher->setKey($key); + $ciphertext = $cipher->encrypt($plaintext); + $hmac = hash_hmac('sha256', $iv . $ciphertext, $key, true); + + return $hmac . $iv . $ciphertext; + } + + /** + * @param string $encrypted + * + * @return string|null + */ + private static function decryptWithPhpseclib($encrypted) + { + $key = self::getEncryptionKey(); + $hmac = mb_substr($encrypted, 0, 32, '8bit'); + $iv = mb_substr($encrypted, 32, 16, '8bit'); + $ciphertext = mb_substr($encrypted, 48, null, '8bit'); + $calculatedHmac = hash_hmac('sha256', $iv . $ciphertext, $key, true); + if (! hash_equals($hmac, $calculatedHmac)) { + return null; + } + + $cipher = new AES(AES::MODE_CBC); + $cipher->setIV($iv); + $cipher->setKey($key); + + return $cipher->decrypt($ciphertext); + } + + /** + * @return string + */ + private static function getEncryptionKey() + { + global $PMA_Config; + + return $_SESSION[' HMAC_secret '] . $PMA_Config->get('blowfish_secret'); + } +} diff --git a/libraries/classes/Url.php b/libraries/classes/Url.php index d63d7b6989..c5c411df0b 100644 --- a/libraries/classes/Url.php +++ b/libraries/classes/Url.php @@ -7,8 +7,7 @@ */ namespace PhpMyAdmin; -use phpseclib\Crypt\AES; -use phpseclib\Crypt\Random; +use PhpMyAdmin\Crypto\Crypto; /** * Static methods for URL/hidden inputs generating @@ -223,7 +222,7 @@ class Url $query = http_build_query($params, null, $separator); - if (isset($params['db'])) { + if ($PMA_Config->get('URLQueryEncryption') && isset($params['db'])) { $encryptedQuery = self::encryptQuery($query); $query = http_build_query(['eq' => $encryptedQuery], null, $separator); } @@ -241,17 +240,7 @@ class Url */ public static function encryptQuery($query) { - global $PMA_Config; - - $key = $_SESSION[' HMAC_secret '] . $PMA_Config->get('blowfish_secret'); - $cipher = new AES(AES::MODE_CBC); - $iv = Random::string(16); - $cipher->setIV($iv); - $cipher->setKey($key); - $ciphertext = $cipher->encrypt($query); - $hmac = hash_hmac('sha256', $iv . $ciphertext, $key, true); - - return strtr(base64_encode($hmac . $iv . $ciphertext), '+/', '-_'); + return strtr(base64_encode(Crypto::encrypt($query)), '+/', '-_'); } /** @@ -260,24 +249,7 @@ class Url */ public static function decryptQuery($query) { - global $PMA_Config; - - $encryptedQuery = base64_decode(strtr($query, '-_', '+/')); - $hmac = mb_substr($encryptedQuery, 0, 32, '8bit'); - $iv = mb_substr($encryptedQuery, 32, 16, '8bit'); - $ciphertext = mb_substr($encryptedQuery, 48, null, '8bit'); - $key = $_SESSION[' HMAC_secret '] . $PMA_Config->get('blowfish_secret'); - $calculated = hash_hmac('sha256', $iv . $ciphertext, $key, true); - - if (! hash_equals($hmac, $calculated)) { - return null; - } - - $cipher = new AES(AES::MODE_CBC); - $cipher->setIV($iv); - $cipher->setKey($key); - - return $cipher->decrypt($ciphertext); + return Crypto::decrypt(base64_decode(strtr($query, '-_', '+/'))); } /** diff --git a/libraries/config.default.php b/libraries/config.default.php index e7cf828bff..fdd7c13639 100644 --- a/libraries/config.default.php +++ b/libraries/config.default.php @@ -817,6 +817,13 @@ $cfg['UseDbSearch'] = true; */ $cfg['IgnoreMultiSubmitErrors'] = false; +/** + * Define whether phpMyAdmin will encrypt sensitive data from the URL query string. + * + * @global bool $cfg['URLQueryEncryption'] + */ +$cfg['URLQueryEncryption'] = true; + /** * allow login to any user entered server in cookie based authentication * diff --git a/test/classes/Config/PageSettingsTest.php b/test/classes/Config/PageSettingsTest.php index 677336d323..98a84cba8d 100644 --- a/test/classes/Config/PageSettingsTest.php +++ b/test/classes/Config/PageSettingsTest.php @@ -26,6 +26,7 @@ class PageSettingsTest extends PmaTestCase public function setUp() { $GLOBALS['PMA_Config'] = new Config(); + $GLOBALS['PMA_Config']->set('URLQueryEncryption', false); $GLOBALS['server'] = 1; $GLOBALS['db'] = 'db'; $GLOBALS['table'] = ''; diff --git a/test/classes/Controllers/Server/ServerDatabasesControllerTest.php b/test/classes/Controllers/Server/ServerDatabasesControllerTest.php index 8ff219ba60..2cfda9f14a 100644 --- a/test/classes/Controllers/Server/ServerDatabasesControllerTest.php +++ b/test/classes/Controllers/Server/ServerDatabasesControllerTest.php @@ -34,6 +34,7 @@ class ServerDatabasesControllerTest extends PmaTestCase //$GLOBALS $GLOBALS['PMA_Config'] = new Config(); + $GLOBALS['PMA_Config']->set('URLQueryEncryption', false); $GLOBALS['PMA_Config']->enableBc(); $GLOBALS['db'] = 'db'; diff --git a/test/classes/Display/ResultsTest.php b/test/classes/Display/ResultsTest.php index f986883aac..fdeed4c78a 100644 --- a/test/classes/Display/ResultsTest.php +++ b/test/classes/Display/ResultsTest.php @@ -44,6 +44,7 @@ class ResultsTest extends PmaTestCase $GLOBALS['PMA_PHP_SELF'] = 'index.php'; $this->object = new DisplayResults('as', '', '', ''); $GLOBALS['PMA_Config'] = new Config(); + $GLOBALS['PMA_Config']->set('URLQueryEncryption', false); $GLOBALS['PMA_Config']->enableBc(); $GLOBALS['text_dir'] = 'ltr'; $_SESSION[' HMAC_secret '] = 'test'; diff --git a/test/classes/FooterTest.php b/test/classes/FooterTest.php index 76704d9e36..cd6dc7f13b 100644 --- a/test/classes/FooterTest.php +++ b/test/classes/FooterTest.php @@ -46,6 +46,7 @@ class FooterTest extends PmaTestCase $GLOBALS['table'] = ''; $GLOBALS['text_dir'] = 'ltr'; $GLOBALS['PMA_Config'] = new Config(); + $GLOBALS['PMA_Config']->set('URLQueryEncryption', false); $GLOBALS['PMA_Config']->enableBc(); $GLOBALS['cfg']['Server']['DisableIS'] = false; $GLOBALS['cfg']['Server']['verbose'] = 'verbose host'; diff --git a/test/classes/InsertEditTest.php b/test/classes/InsertEditTest.php index d274ca59b5..8ce35542da 100644 --- a/test/classes/InsertEditTest.php +++ b/test/classes/InsertEditTest.php @@ -62,6 +62,7 @@ class InsertEditTest extends TestCase $GLOBALS['cfg']['LoginCookieValidity'] = 1440; $GLOBALS['cfg']['enable_drag_drop_import'] = true; $GLOBALS['PMA_Config'] = new Config(); + $GLOBALS['PMA_Config']->set('URLQueryEncryption', false); $this->insertEdit = new InsertEdit($GLOBALS['dbi']); } diff --git a/test/classes/Navigation/NavigationTreeTest.php b/test/classes/Navigation/NavigationTreeTest.php index 0149332035..f391db0459 100644 --- a/test/classes/Navigation/NavigationTreeTest.php +++ b/test/classes/Navigation/NavigationTreeTest.php @@ -43,6 +43,7 @@ class NavigationTreeTest extends PmaTestCase { $GLOBALS['server'] = 1; $GLOBALS['PMA_Config'] = new Config(); + $GLOBALS['PMA_Config']->set('URLQueryEncryption', false); $GLOBALS['PMA_Config']->enableBc(); $GLOBALS['cfg']['Server']['host'] = 'localhost'; $GLOBALS['cfg']['Server']['user'] = 'root'; diff --git a/test/classes/Plugins/Auth/AuthenticationCookieTest.php b/test/classes/Plugins/Auth/AuthenticationCookieTest.php index fc5e034c20..060d0a0945 100644 --- a/test/classes/Plugins/Auth/AuthenticationCookieTest.php +++ b/test/classes/Plugins/Auth/AuthenticationCookieTest.php @@ -37,6 +37,7 @@ class AuthenticationCookieTest extends PmaTestCase function setUp() { $GLOBALS['PMA_Config'] = new Config(); + $GLOBALS['PMA_Config']->set('URLQueryEncryption', false); $GLOBALS['PMA_Config']->enableBc(); $GLOBALS['server'] = 0; $GLOBALS['text_dir'] = 'ltr'; diff --git a/test/classes/PmaTestCase.php b/test/classes/PmaTestCase.php index b2e72b7a51..889958347d 100644 --- a/test/classes/PmaTestCase.php +++ b/test/classes/PmaTestCase.php @@ -28,6 +28,7 @@ class PmaTestCase extends TestCase { require 'libraries/config.default.php'; $GLOBALS['cfg'] = $cfg; + $GLOBALS['PMA_Config']->set('URLQueryEncryption', false); } /** From 3c11d62b31acdc994ae426999b5cf639cbdd32c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maur=C3=ADcio=20Meneghini=20Fauth?= Date: Sat, 16 Oct 2021 14:23:02 -0300 Subject: [PATCH 04/13] Add support for Sodium encryption MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Maurício Meneghini Fauth --- composer.json | 7 ++- doc/config.rst | 10 +++ libraries/classes/Crypto/Crypto.php | 95 +++++++++++++++++++++++++---- libraries/classes/Url.php | 8 ++- libraries/config.default.php | 7 +++ 5 files changed, 111 insertions(+), 16 deletions(-) diff --git a/composer.json b/composer.json index c9fcfd9453..d5c178e0ee 100644 --- a/composer.json +++ b/composer.json @@ -76,7 +76,8 @@ "tecnickcom/tcpdf": "For PDF support", "pragmarx/google2fa": "For 2FA authentication", "bacon/bacon-qr-code": "For 2FA authentication", - "samyoul/u2f-php-server": "For FIDO U2F authentication" + "samyoul/u2f-php-server": "For FIDO U2F authentication", + "paragonie/sodium_compat": "For modern encryption support" }, "require-dev": { "phpunit/phpunit": "^4.8.36 || ^5.7", @@ -87,7 +88,9 @@ "pragmarx/google2fa": "^3.0", "bacon/bacon-qr-code": "^1.0", "samyoul/u2f-php-server": "^1.1", - "phpmyadmin/coding-standard": "^0.3" + "phpmyadmin/coding-standard": "^0.3", + "paragonie/random_compat": ">=2", + "paragonie/sodium_compat": "^1.17" }, "extra": { "branch-alias": { diff --git a/doc/config.rst b/doc/config.rst index f0b9ae2218..e2e7eefb01 100644 --- a/doc/config.rst +++ b/doc/config.rst @@ -1747,6 +1747,16 @@ Generic settings and table name) from the URL query string. Default is to encrypt the URL query string. +.. config:option:: $cfg['URLQueryEncryptionSecretKey'] + + :type: string + :default: ``''`` + + .. versionadded:: 4.9.8 + + A secret key used to encrypt/decrypt the URL query string. + Should be 32 bytes long. + Cookie authentication options ----------------------------- diff --git a/libraries/classes/Crypto/Crypto.php b/libraries/classes/Crypto/Crypto.php index 128598639d..5f12fbdcae 100644 --- a/libraries/classes/Crypto/Crypto.php +++ b/libraries/classes/Crypto/Crypto.php @@ -7,14 +7,34 @@ use phpseclib\Crypt\Random; final class Crypto { + /** @var bool */ + private $hasRandomBytesSupport; + + /** @var bool */ + private $hasSodiumSupport; + + public function __construct() + { + $this->hasRandomBytesSupport = is_callable('random_bytes'); + $this->hasSodiumSupport = $this->hasRandomBytesSupport + && is_callable('sodium_crypto_secretbox') + && is_callable('sodium_crypto_secretbox_open') + && defined('SODIUM_CRYPTO_SECRETBOX_NONCEBYTES') + && defined('SODIUM_CRYPTO_SECRETBOX_KEYBYTES'); + } + /** * @param string $plaintext * * @return string */ - public static function encrypt($plaintext) + public function encrypt($plaintext) { - return self::encryptWithPhpseclib($plaintext); + if ($this->hasSodiumSupport) { + return $this->encryptWithSodium($plaintext); + } + + return $this->encryptWithPhpseclib($plaintext); } /** @@ -22,9 +42,38 @@ final class Crypto * * @return string */ - public static function decrypt($ciphertext) + public function decrypt($ciphertext) { - return self::decryptWithPhpseclib($ciphertext); + if ($this->hasSodiumSupport) { + return $this->decryptWithSodium($ciphertext); + } + + return $this->decryptWithPhpseclib($ciphertext); + } + + /** + * @return string + */ + private function getEncryptionKey() + { + global $PMA_Config; + + $keyLength = $this->hasSodiumSupport ? SODIUM_CRYPTO_SECRETBOX_KEYBYTES : 32; + + $key = $PMA_Config->get('URLQueryEncryptionSecretKey'); + if (is_string($key) && mb_strlen($key, '8bit') === $keyLength) { + return $key; + } + + $key = isset($_SESSION['URLQueryEncryptionSecretKey']) ? $_SESSION['URLQueryEncryptionSecretKey'] : null; + if (is_string($key) && mb_strlen($key, '8bit') === $keyLength) { + return $key; + } + + $key = $this->hasRandomBytesSupport ? random_bytes($keyLength) : Random::string($keyLength); + $_SESSION['URLQueryEncryptionSecretKey'] = $key; + + return $key; } /** @@ -32,11 +81,11 @@ final class Crypto * * @return string */ - private static function encryptWithPhpseclib($plaintext) + private function encryptWithPhpseclib($plaintext) { - $key = self::getEncryptionKey(); + $key = $this->getEncryptionKey(); $cipher = new AES(AES::MODE_CBC); - $iv = Random::string(16); + $iv = $this->hasRandomBytesSupport ? random_bytes(16) : Random::string(16); $cipher->setIV($iv); $cipher->setKey($key); $ciphertext = $cipher->encrypt($plaintext); @@ -50,9 +99,9 @@ final class Crypto * * @return string|null */ - private static function decryptWithPhpseclib($encrypted) + private function decryptWithPhpseclib($encrypted) { - $key = self::getEncryptionKey(); + $key = $this->getEncryptionKey(); $hmac = mb_substr($encrypted, 0, 32, '8bit'); $iv = mb_substr($encrypted, 32, 16, '8bit'); $ciphertext = mb_substr($encrypted, 48, null, '8bit'); @@ -69,12 +118,34 @@ final class Crypto } /** + * @param string $plaintext + * * @return string */ - private static function getEncryptionKey() + private function encryptWithSodium($plaintext) { - global $PMA_Config; + $key = $this->getEncryptionKey(); + $nonce = random_bytes(SODIUM_CRYPTO_SECRETBOX_NONCEBYTES); + $ciphertext = sodium_crypto_secretbox($plaintext, $nonce, $key); - return $_SESSION[' HMAC_secret '] . $PMA_Config->get('blowfish_secret'); + return $nonce . $ciphertext; + } + + /** + * @param string $encrypted + * + * @return string|null + */ + private function decryptWithSodium($encrypted) + { + $key = $this->getEncryptionKey(); + $nonce = mb_substr($encrypted, 0, SODIUM_CRYPTO_SECRETBOX_NONCEBYTES, '8bit'); + $ciphertext = mb_substr($encrypted, SODIUM_CRYPTO_SECRETBOX_NONCEBYTES, null, '8bit'); + $decrypted = sodium_crypto_secretbox_open($ciphertext, $nonce, $key); + if ($decrypted === false) { + return null; + } + + return $decrypted; } } diff --git a/libraries/classes/Url.php b/libraries/classes/Url.php index c5c411df0b..b5594b38ed 100644 --- a/libraries/classes/Url.php +++ b/libraries/classes/Url.php @@ -240,7 +240,9 @@ class Url */ public static function encryptQuery($query) { - return strtr(base64_encode(Crypto::encrypt($query)), '+/', '-_'); + $crypto = new Crypto(); + + return strtr(base64_encode($crypto->encrypt($query)), '+/', '-_'); } /** @@ -249,7 +251,9 @@ class Url */ public static function decryptQuery($query) { - return Crypto::decrypt(base64_decode(strtr($query, '-_', '+/'))); + $crypto = new Crypto(); + + return $crypto->decrypt(base64_decode(strtr($query, '-_', '+/'))); } /** diff --git a/libraries/config.default.php b/libraries/config.default.php index fdd7c13639..b81981ba45 100644 --- a/libraries/config.default.php +++ b/libraries/config.default.php @@ -824,6 +824,13 @@ $cfg['IgnoreMultiSubmitErrors'] = false; */ $cfg['URLQueryEncryption'] = true; +/** + * A secret key used to encrypt/decrypt the URL query string. Should be 32 bytes long. + * + * @global string $cfg['URLQueryEncryptionSecretKey'] + */ +$cfg['URLQueryEncryptionSecretKey'] = ''; + /** * allow login to any user entered server in cookie based authentication * From f1aaec1a5a3518bb4312c0dea540caa7c49d8ffe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maur=C3=ADcio=20Meneghini=20Fauth?= Date: Sat, 16 Oct 2021 16:01:38 -0300 Subject: [PATCH 05/13] Add `PhpMyAdmin\Url::buildHttpQuery` method MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Maurício Meneghini Fauth --- libraries/classes/ErrorReport.php | 1 + libraries/classes/Url.php | 54 +++++++++++++++++++++++++++---- libraries/common.inc.php | 2 +- 3 files changed, 49 insertions(+), 8 deletions(-) diff --git a/libraries/classes/ErrorReport.php b/libraries/classes/ErrorReport.php index 6a3e65a230..3abef4815d 100644 --- a/libraries/classes/ErrorReport.php +++ b/libraries/classes/ErrorReport.php @@ -197,6 +197,7 @@ class ErrorReport unset($queryArray["table"]); unset($queryArray["token"]); unset($queryArray["server"]); + unset($queryArray["eq"]); $query = http_build_query($queryArray); } else { $query = ''; diff --git a/libraries/classes/Url.php b/libraries/classes/Url.php index b5594b38ed..9cabee652c 100644 --- a/libraries/classes/Url.php +++ b/libraries/classes/Url.php @@ -205,7 +205,6 @@ class Url { /** @var Config $PMA_Config */ global $PMA_Config; - $separator = Url::getArgSeparator(); // avoid overwriting when creating navi panel links to servers if (isset($GLOBALS['server']) @@ -220,12 +219,7 @@ class Url $params['lang'] = $GLOBALS['lang']; } - $query = http_build_query($params, null, $separator); - - if ($PMA_Config->get('URLQueryEncryption') && isset($params['db'])) { - $encryptedQuery = self::encryptQuery($query); - $query = http_build_query(['eq' => $encryptedQuery], null, $separator); - } + $query = self::buildHttpQuery($params); if ($divider != '?' || strlen($query) > 0) { return $divider . $query; @@ -234,6 +228,52 @@ class Url return ''; } + /** + * @param array $params + * @return string + */ + private static function buildHttpQuery($params) + { + global $PMA_Config; + + $separator = self::getArgSeparator(); + + if (! $PMA_Config->get('URLQueryEncryption')) { + return http_build_query($params, null, $separator); + } + + $data = $params; + $keys = [ + 'db', + 'table', + 'sql_query', + 'sql_signature', + 'where_clause', + 'goto', + 'back', + 'message_to_show', + 'username', + 'hostname', + 'dbname', + 'tablename', + 'checkprivsdb', + 'checkprivstable', + ]; + $paramsToEncrypt = []; + foreach ($params as $paramKey => $paramValue) { + if (! in_array($paramKey, $keys)) { + continue; + } + + $paramsToEncrypt[$paramKey] = $paramValue; + unset($data[$paramKey]); + } + + $data['eq'] = self::encryptQuery(json_encode($paramsToEncrypt)); + + return http_build_query($data, null, $separator); + } + /** * @param string $query * @return string diff --git a/libraries/common.inc.php b/libraries/common.inc.php index fafc6d7353..0277c8af55 100644 --- a/libraries/common.inc.php +++ b/libraries/common.inc.php @@ -136,7 +136,7 @@ if (! defined('PMA_NO_SESSION')) { if (isset($_GET['eq']) && is_string($_GET['eq'])) { $decryptedQuery = Url::decryptQuery($_GET['eq']); if ($decryptedQuery !== null) { - parse_str($decryptedQuery, $urlQueryParams); + $urlQueryParams = json_decode($decryptedQuery); foreach ($urlQueryParams as $urlQueryParamKey => $urlQueryParamValue) { $_GET[$urlQueryParamKey] = $urlQueryParamValue; $_REQUEST[$urlQueryParamKey] = $urlQueryParamValue; From 3095181bb3449401bcae92cbb1d36762a70f5a27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maur=C3=ADcio=20Meneghini=20Fauth?= Date: Sat, 16 Oct 2021 16:51:11 -0300 Subject: [PATCH 06/13] Encrypt query params of Navigation links MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Maurício Meneghini Fauth --- .../classes/Navigation/NavigationTree.php | 23 +++++++++++++++++-- libraries/classes/Url.php | 3 ++- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/libraries/classes/Navigation/NavigationTree.php b/libraries/classes/Navigation/NavigationTree.php index 78659a4b7f..c40b1408b9 100644 --- a/libraries/classes/Navigation/NavigationTree.php +++ b/libraries/classes/Navigation/NavigationTree.php @@ -1156,7 +1156,7 @@ class NavigationTree } foreach ($icons as $key => $icon) { - $link = vsprintf($iconLinks[$key], $args); + $link = $this->encryptQueryParams(vsprintf($iconLinks[$key], $args)); if ($linkClass != '') { $retval .= ""; $retval .= "{$icon}"; @@ -1174,7 +1174,7 @@ class NavigationTree foreach ($node->parents(true) as $parent) {; $args[] = urlencode($parent->real_name); } - $link = vsprintf($node->links['text'], $args); + $link = $this->encryptQueryParams(vsprintf($node->links['text'], $args)); $title = isset($node->links['title']) ? $node->links['title'] : ''; if ($node->type == Node::CONTAINER) { $retval .= " "; @@ -1557,4 +1557,23 @@ class NavigationTree return $retval; } + + /** + * @param string $link + * + * @return string + */ + private function encryptQueryParams($link) + { + global $PMA_Config; + + if (! $PMA_Config->get('URLQueryEncryption')) { + return $link; + } + + $url = parse_url($link); + parse_str(htmlspecialchars_decode($url['query']), $query); + + return $url['path'] . '?' . Url::buildHttpQuery($query); + } } diff --git a/libraries/classes/Url.php b/libraries/classes/Url.php index 9cabee652c..215a1b98fc 100644 --- a/libraries/classes/Url.php +++ b/libraries/classes/Url.php @@ -232,7 +232,7 @@ class Url * @param array $params * @return string */ - private static function buildHttpQuery($params) + public static function buildHttpQuery($params) { global $PMA_Config; @@ -246,6 +246,7 @@ class Url $keys = [ 'db', 'table', + 'field', 'sql_query', 'sql_signature', 'where_clause', From d057b68aa1136f5b37e63fbeb2fee46c211ae9fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maur=C3=ADcio=20Meneghini=20Fauth?= Date: Sun, 17 Oct 2021 00:29:39 -0300 Subject: [PATCH 07/13] Add unit tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Maurício Meneghini Fauth --- libraries/classes/Core.php | 23 ++++ libraries/classes/Crypto/Crypto.php | 10 +- .../classes/Navigation/NavigationTree.php | 2 +- libraries/classes/Url.php | 4 +- libraries/common.inc.php | 12 +- test/classes/CoreTest.php | 59 +++++++++ test/classes/Crypto/CryptoTest.php | 114 ++++++++++++++++++ .../classes/Navigation/NavigationTreeTest.php | 35 ++++++ test/classes/UrlTest.php | 58 +++++++++ 9 files changed, 301 insertions(+), 16 deletions(-) create mode 100644 test/classes/Crypto/CryptoTest.php diff --git a/libraries/classes/Core.php b/libraries/classes/Core.php index bf5edde41a..98ea2031f6 100644 --- a/libraries/classes/Core.php +++ b/libraries/classes/Core.php @@ -1317,4 +1317,27 @@ class Core $hmac = hash_hmac('sha256', $sqlQuery, $_SESSION[' HMAC_secret '] . $cfg['blowfish_secret']); return hash_equals($hmac, $signature); } + + /** + * @return void + */ + public static function populateRequestWithEncryptedQueryParams() + { + if (! isset($_GET['eq']) || ! is_string($_GET['eq'])) { + unset($_GET['eq'], $_REQUEST['eq']); + return; + } + + $decryptedQuery = Url::decryptQuery($_GET['eq']); + unset($_GET['eq'], $_REQUEST['eq']); + if ($decryptedQuery === null) { + return; + } + + $urlQueryParams = (array) json_decode($decryptedQuery); + foreach ($urlQueryParams as $urlQueryParamKey => $urlQueryParamValue) { + $_GET[$urlQueryParamKey] = $urlQueryParamValue; + $_REQUEST[$urlQueryParamKey] = $urlQueryParamValue; + } + } } diff --git a/libraries/classes/Crypto/Crypto.php b/libraries/classes/Crypto/Crypto.php index 5f12fbdcae..6449f7dca5 100644 --- a/libraries/classes/Crypto/Crypto.php +++ b/libraries/classes/Crypto/Crypto.php @@ -13,10 +13,14 @@ final class Crypto /** @var bool */ private $hasSodiumSupport; - public function __construct() + /** + * @param bool $forceFallback Force the usage of the fallback functions. + */ + public function __construct($forceFallback = false) { - $this->hasRandomBytesSupport = is_callable('random_bytes'); - $this->hasSodiumSupport = $this->hasRandomBytesSupport + $this->hasRandomBytesSupport = ! $forceFallback && is_callable('random_bytes'); + $this->hasSodiumSupport = ! $forceFallback + && $this->hasRandomBytesSupport && is_callable('sodium_crypto_secretbox') && is_callable('sodium_crypto_secretbox_open') && defined('SODIUM_CRYPTO_SECRETBOX_NONCEBYTES') diff --git a/libraries/classes/Navigation/NavigationTree.php b/libraries/classes/Navigation/NavigationTree.php index c40b1408b9..3d37577d95 100644 --- a/libraries/classes/Navigation/NavigationTree.php +++ b/libraries/classes/Navigation/NavigationTree.php @@ -1574,6 +1574,6 @@ class NavigationTree $url = parse_url($link); parse_str(htmlspecialchars_decode($url['query']), $query); - return $url['path'] . '?' . Url::buildHttpQuery($query); + return $url['path'] . '?' . htmlspecialchars(Url::buildHttpQuery($query)); } } diff --git a/libraries/classes/Url.php b/libraries/classes/Url.php index 215a1b98fc..cd1f89a4c0 100644 --- a/libraries/classes/Url.php +++ b/libraries/classes/Url.php @@ -270,7 +270,9 @@ class Url unset($data[$paramKey]); } - $data['eq'] = self::encryptQuery(json_encode($paramsToEncrypt)); + if ($paramsToEncrypt !== []) { + $data['eq'] = self::encryptQuery(json_encode($paramsToEncrypt)); + } return http_build_query($data, null, $separator); } diff --git a/libraries/common.inc.php b/libraries/common.inc.php index 0277c8af55..6705fd08be 100644 --- a/libraries/common.inc.php +++ b/libraries/common.inc.php @@ -43,7 +43,6 @@ use PhpMyAdmin\Response; use PhpMyAdmin\Session; use PhpMyAdmin\ThemeManager; use PhpMyAdmin\Tracker; -use PhpMyAdmin\Url; use PhpMyAdmin\Util; /** @@ -133,16 +132,7 @@ if (! defined('PMA_NO_SESSION')) { Session::setUp($GLOBALS['PMA_Config'], $GLOBALS['error_handler']); } -if (isset($_GET['eq']) && is_string($_GET['eq'])) { - $decryptedQuery = Url::decryptQuery($_GET['eq']); - if ($decryptedQuery !== null) { - $urlQueryParams = json_decode($decryptedQuery); - foreach ($urlQueryParams as $urlQueryParamKey => $urlQueryParamValue) { - $_GET[$urlQueryParamKey] = $urlQueryParamValue; - $_REQUEST[$urlQueryParamKey] = $urlQueryParamValue; - } - } -} +Core::populateRequestWithEncryptedQueryParams(); /** * init some variables LABEL_variables_init diff --git a/test/classes/CoreTest.php b/test/classes/CoreTest.php index 11e29fb90f..87a1525c3f 100644 --- a/test/classes/CoreTest.php +++ b/test/classes/CoreTest.php @@ -11,6 +11,7 @@ use PhpMyAdmin\Config; use PhpMyAdmin\Core; use PhpMyAdmin\Sanitize; use PhpMyAdmin\Tests\PmaTestCase; +use PhpMyAdmin\Url; use stdClass; /** @@ -50,6 +51,7 @@ class CoreTest extends PmaTestCase $GLOBALS['db'] = ''; $GLOBALS['table'] = ''; $GLOBALS['PMA_PHP_SELF'] = 'http://example.net/'; + $GLOBALS['PMA_Config']->set('URLQueryEncryption', false); } /** @@ -1226,4 +1228,61 @@ class CoreTest extends PmaTestCase // Must work now, (good secret and blowfish_secret) $this->assertTrue(Core::checkSqlQuerySignature($sqlQuery, $hmac)); } + + /** + * @return void + */ + public function testPopulateRequestWithEncryptedQueryParams() + { + global $PMA_Config; + + $_SESSION = []; + $PMA_Config->set('URLQueryEncryption', true); + $PMA_Config->set('URLQueryEncryptionSecretKey', str_repeat('a', 32)); + + $_GET = ['pos' => '0', 'eq' => Url::encryptQuery('{"db":"test_db","table":"test_table"}')]; + $_REQUEST = $_GET; + + Core::populateRequestWithEncryptedQueryParams(); + + $expected = ['pos' => '0', 'db' => 'test_db', 'table' => 'test_table']; + + $this->assertEquals($expected, $_GET); + $this->assertEquals($expected, $_REQUEST); + } + + /** + * @return void + * + * @dataProvider providerForTestPopulateRequestWithEncryptedQueryParamsWithInvalidParam + */ + public function testPopulateRequestWithEncryptedQueryParamsWithInvalidParam($encrypted, $decrypted) + { + global $PMA_Config; + + $_SESSION = []; + $PMA_Config->set('URLQueryEncryption', true); + $PMA_Config->set('URLQueryEncryptionSecretKey', str_repeat('a', 32)); + + $_GET = $encrypted; + $_REQUEST = $encrypted; + + Core::populateRequestWithEncryptedQueryParams(); + + $this->assertEquals($decrypted, $_GET); + $this->assertEquals($decrypted, $_REQUEST); + } + + /** + * @return string[][][] + */ + public function providerForTestPopulateRequestWithEncryptedQueryParamsWithInvalidParam() + { + return [ + [[], []], + [['eq' => []], []], + [['eq' => ''], []], + [['eq' => 'invalid'], []], + ]; + } } diff --git a/test/classes/Crypto/CryptoTest.php b/test/classes/Crypto/CryptoTest.php new file mode 100644 index 0000000000..a73260d07b --- /dev/null +++ b/test/classes/Crypto/CryptoTest.php @@ -0,0 +1,114 @@ +set('URLQueryEncryptionSecretKey', str_repeat('a', 32)); + + $cryptoWithSodium = new Crypto(); + $encrypted = $cryptoWithSodium->encrypt('test'); + $this->assertNotSame('test', $encrypted); + $this->assertSame('test', $cryptoWithSodium->decrypt($encrypted)); + $this->assertArrayNotHasKey('URLQueryEncryptionSecretKey', $_SESSION); + + $cryptoWithPhpseclib = new Crypto(true); + $encrypted = $cryptoWithPhpseclib->encrypt('test'); + $this->assertNotSame('test', $encrypted); + $this->assertSame('test', $cryptoWithPhpseclib->decrypt($encrypted)); + $this->assertArrayNotHasKey('URLQueryEncryptionSecretKey', $_SESSION); + } + + /** + * @return void + */ + public function testWithValidKeyFromSession() + { + global $PMA_Config; + + $_SESSION = ['URLQueryEncryptionSecretKey' => str_repeat('a', 32)]; + $PMA_Config->set('URLQueryEncryptionSecretKey', ''); + + $cryptoWithSodium = new Crypto(); + $encrypted = $cryptoWithSodium->encrypt('test'); + $this->assertNotSame('test', $encrypted); + $this->assertSame('test', $cryptoWithSodium->decrypt($encrypted)); + $this->assertArrayHasKey('URLQueryEncryptionSecretKey', $_SESSION); + + $cryptoWithPhpseclib = new Crypto(true); + $encrypted = $cryptoWithPhpseclib->encrypt('test'); + $this->assertNotSame('test', $encrypted); + $this->assertSame('test', $cryptoWithPhpseclib->decrypt($encrypted)); + $this->assertArrayHasKey('URLQueryEncryptionSecretKey', $_SESSION); + } + + /** + * @return void + */ + public function testWithNewSessionKey() + { + global $PMA_Config; + + $_SESSION = []; + $PMA_Config->set('URLQueryEncryptionSecretKey', ''); + + $cryptoWithSodium = new Crypto(); + $encrypted = $cryptoWithSodium->encrypt('test'); + $this->assertNotSame('test', $encrypted); + $this->assertSame('test', $cryptoWithSodium->decrypt($encrypted)); + $this->assertArrayHasKey('URLQueryEncryptionSecretKey', $_SESSION); + $this->assertEquals(32, mb_strlen($_SESSION['URLQueryEncryptionSecretKey'], '8bit')); + + $cryptoWithPhpseclib = new Crypto(true); + $encrypted = $cryptoWithPhpseclib->encrypt('test'); + $this->assertNotSame('test', $encrypted); + $this->assertSame('test', $cryptoWithPhpseclib->decrypt($encrypted)); + $this->assertArrayHasKey('URLQueryEncryptionSecretKey', $_SESSION); + $this->assertEquals(32, mb_strlen($_SESSION['URLQueryEncryptionSecretKey'], '8bit')); + } + + /** + * @return void + */ + public function testDecryptWithInvalidKey() + { + global $PMA_Config; + + $_SESSION = []; + $PMA_Config->set('URLQueryEncryptionSecretKey', str_repeat('a', 32)); + + $cryptoWithSodium = new Crypto(); + $encrypted = $cryptoWithSodium->encrypt('test'); + $this->assertNotSame('test', $encrypted); + $this->assertSame('test', $cryptoWithSodium->decrypt($encrypted)); + + $PMA_Config->set('URLQueryEncryptionSecretKey', str_repeat('b', 32)); + + $cryptoWithSodium = new Crypto(); + $this->assertNull($cryptoWithSodium->decrypt($encrypted)); + + $cryptoWithPhpseclib = new Crypto(true); + $encrypted = $cryptoWithPhpseclib->encrypt('test'); + $this->assertNotSame('test', $encrypted); + $this->assertSame('test', $cryptoWithPhpseclib->decrypt($encrypted)); + + $PMA_Config->set('URLQueryEncryptionSecretKey', str_repeat('a', 32)); + + $cryptoWithPhpseclib = new Crypto(true); + $this->assertNull($cryptoWithPhpseclib->decrypt($encrypted)); + } +} diff --git a/test/classes/Navigation/NavigationTreeTest.php b/test/classes/Navigation/NavigationTreeTest.php index f391db0459..963019fb8a 100644 --- a/test/classes/Navigation/NavigationTreeTest.php +++ b/test/classes/Navigation/NavigationTreeTest.php @@ -11,6 +11,8 @@ use PhpMyAdmin\Config; use PhpMyAdmin\Navigation\NavigationTree; use PhpMyAdmin\Tests\PmaTestCase; use PhpMyAdmin\Theme; +use PhpMyAdmin\Url; +use ReflectionMethod; /* * we must set $GLOBALS['server'] here @@ -101,4 +103,37 @@ class NavigationTreeTest extends PmaTestCase $result = $this->object->renderDbSelect(); $this->assertContains('pma_navigation_select_database', $result); } + + /** + * @return void + */ + public function testEncryptQueryParams() + { + global $PMA_Config; + + $_SESSION = []; + $PMA_Config->set('URLQueryEncryption', false); + $PMA_Config->set('URLQueryEncryptionSecretKey', str_repeat('a', 32)); + + $method = new ReflectionMethod($this->object, 'encryptQueryParams'); + $method->setAccessible(true); + + $link = 'tbl_structure.php?server=1&db=test_db&table=test_table&pos=0'; + + $actual = $method->invoke($this->object, $link); + $this->assertEquals($link, $actual); + + $PMA_Config->set('URLQueryEncryption', true); + + $actual = $method->invoke($this->object, $link); + $this->assertStringStartsWith('tbl_structure.php?server=1&pos=0&eq=', $actual); + + $url = parse_url($actual); + parse_str(htmlspecialchars_decode($url['query']), $query); + + $this->assertRegExp('/^[a-zA-Z0-9-_=]+$/', $query['eq']); + $decrypted = Url::decryptQuery($query['eq']); + $this->assertJson($decrypted); + $this->assertSame('{"db":"test_db","table":"test_table"}', $decrypted); + } } diff --git a/test/classes/UrlTest.php b/test/classes/UrlTest.php index 2bce450315..737e54aa41 100644 --- a/test/classes/UrlTest.php +++ b/test/classes/UrlTest.php @@ -27,6 +27,7 @@ class UrlTest extends TestCase public function setUp() { unset($_COOKIE['pma_lang']); + $GLOBALS['PMA_Config']->set('URLQueryEncryption', false); } /** @@ -105,4 +106,61 @@ class UrlTest extends TestCase $expected = '?server=x' . htmlentities($separator) . 'lang=en' ; $this->assertEquals($expected, Url::getCommon()); } + + /** + * @return void + */ + public function testBuildHttpQueryWithUrlQueryEncryptionDisabled() + { + global $PMA_Config; + + $PMA_Config->set('URLQueryEncryption', false); + $params = ['db' => 'test_db', 'table' => 'test_table', 'pos' => 0]; + $this->assertEquals('db=test_db&table=test_table&pos=0', Url::buildHttpQuery($params)); + } + + /** + * @return void + */ + public function testBuildHttpQueryWithUrlQueryEncryptionEnabled() + { + global $PMA_Config; + + $_SESSION = []; + $PMA_Config->set('URLQueryEncryption', true); + $PMA_Config->set('URLQueryEncryptionSecretKey', str_repeat('a', 32)); + + $params = ['db' => 'test_db', 'table' => 'test_table', 'pos' => 0]; + $query = Url::buildHttpQuery($params); + $this->assertStringStartsWith('pos=0&eq=', $query); + parse_str($query, $queryParams); + $this->assertCount(2, $queryParams); + $this->assertSame('0', $queryParams['pos']); + $this->assertTrue(is_string($queryParams['eq'])); + $this->assertNotSame('', $queryParams['eq']); + $this->assertRegExp('/^[a-zA-Z0-9-_=]+$/', $queryParams['eq']); + $decrypted = Url::decryptQuery($queryParams['eq']); + $this->assertJson($decrypted); + $this->assertSame('{"db":"test_db","table":"test_table"}', $decrypted); + } + + /** + * @return void + */ + public function testQueryEncryption() + { + global $PMA_Config; + + $_SESSION = []; + $PMA_Config->set('URLQueryEncryption', true); + $PMA_Config->set('URLQueryEncryptionSecretKey', str_repeat('a', 32)); + + $query = '{"db":"test_db","table":"test_table"}'; + $encrypted = Url::encryptQuery($query); + $this->assertNotSame($query, $encrypted); + $this->assertNotSame('', $encrypted); + $this->assertRegExp('/^[a-zA-Z0-9-_=]+$/', $encrypted); + $decrypted = Url::decryptQuery($encrypted); + $this->assertSame($query, $decrypted); + } } From 6f0d19f394905645e09c0244f7823b3c4add3125 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maur=C3=ADcio=20Meneghini=20Fauth?= Date: Sun, 17 Oct 2021 16:23:30 -0300 Subject: [PATCH 08/13] Do not encrypt params if used in data-post attribute MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Maurício Meneghini Fauth --- .../Server/ServerBinlogController.php | 6 +++--- libraries/classes/Export.php | 6 +++--- libraries/classes/Index.php | 2 +- libraries/classes/InsertEdit.php | 6 +++--- libraries/classes/Navigation/Navigation.php | 2 +- .../classes/Navigation/Nodes/NodeDatabase.php | 2 +- .../Navigation/Nodes/NodeDatabaseChild.php | 2 +- libraries/classes/ReplicationGui.php | 18 +++++++++--------- libraries/classes/Server/Privileges.php | 2 +- libraries/classes/Server/Status/Processes.php | 6 +++--- libraries/classes/Server/UserGroups.php | 6 +++--- libraries/classes/Sql.php | 2 +- libraries/classes/Tracking.php | 6 +++--- libraries/classes/Url.php | 18 ++++++++++++------ setup/frames/index.inc.php | 2 +- .../database/tracking/tracked_tables.twig | 8 ++++---- .../server/databases/databases_footer.twig | 2 +- templates/table/search/input_box.twig | 2 +- test/classes/Server/Status/ProcessesTest.php | 2 +- test/classes/Server/UserGroupsTest.php | 6 +++--- 20 files changed, 56 insertions(+), 50 deletions(-) diff --git a/libraries/classes/Controllers/Server/ServerBinlogController.php b/libraries/classes/Controllers/Server/ServerBinlogController.php index 57983ec3d2..807b7223bf 100644 --- a/libraries/classes/Controllers/Server/ServerBinlogController.php +++ b/libraries/classes/Controllers/Server/ServerBinlogController.php @@ -193,7 +193,7 @@ class ServerBinlogController extends Controller } $html .= ''; } else { @@ -215,7 +215,7 @@ class ServerBinlogController extends Controller $tempTitle = __('Show Full Queries'); $tempImgMode = 'full'; } - $html .= '' . '' . $tempTitle . ''; @@ -226,7 +226,7 @@ class ServerBinlogController extends Controller $this_url_params = $url_params; $this_url_params['pos'] = $pos + $GLOBALS['cfg']['MaxRows']; $html .= ' - '; diff --git a/libraries/classes/Export.php b/libraries/classes/Export.php index e4d028b224..affcc00af2 100644 --- a/libraries/classes/Export.php +++ b/libraries/classes/Export.php @@ -479,14 +479,14 @@ class Export */ $back_button = '

[ ' . ' ' . Util::getIcon('b_edit', __('Edit')) . '' . '' . "\n"; $this_params = $GLOBALS['url_params']; diff --git a/libraries/classes/InsertEdit.php b/libraries/classes/InsertEdit.php index ad692f0509..77cddf7320 100644 --- a/libraries/classes/InsertEdit.php +++ b/libraries/classes/InsertEdit.php @@ -280,12 +280,12 @@ class InsertEdit if (! $is_show) { return ' : ' + . Url::getCommon($this_url_params, '', false) . '">' . $this->showTypeOrFunctionLabel($which) . ''; } return '' . $this->showTypeOrFunctionLabel($which) . ''; @@ -857,7 +857,7 @@ class InsertEdit 'rownumber' => $rownumber, 'data' => $data ), - '' + '', false ) . '">' . str_replace("'", "\'", $titles['Browse']) . ''; return $html_output; diff --git a/libraries/classes/Navigation/Navigation.php b/libraries/classes/Navigation/Navigation.php index 486bd2f74c..145167a45a 100644 --- a/libraries/classes/Navigation/Navigation.php +++ b/libraries/classes/Navigation/Navigation.php @@ -231,7 +231,7 @@ class Navigation $html .= ''; $html .= '' . htmlspecialchars($hiddenItem) . ''; $html .= '' . Util::getIcon('show', __('Show')) . ''; diff --git a/libraries/classes/Navigation/Nodes/NodeDatabase.php b/libraries/classes/Navigation/Nodes/NodeDatabase.php index d9828a02f6..0844f56380 100644 --- a/libraries/classes/Navigation/Nodes/NodeDatabase.php +++ b/libraries/classes/Navigation/Nodes/NodeDatabase.php @@ -678,7 +678,7 @@ class NodeDatabase extends Node ); $ret = '' . '' . Util::getImage( 'show', diff --git a/libraries/classes/Navigation/Nodes/NodeDatabaseChild.php b/libraries/classes/Navigation/Nodes/NodeDatabaseChild.php index ecae8fab2d..e562af0b74 100644 --- a/libraries/classes/Navigation/Nodes/NodeDatabaseChild.php +++ b/libraries/classes/Navigation/Nodes/NodeDatabaseChild.php @@ -49,7 +49,7 @@ abstract class NodeDatabaseChild extends Node $ret = '' . '' . Util::getImage('hide', __('Hide')) . ''; diff --git a/libraries/classes/ReplicationGui.php b/libraries/classes/ReplicationGui.php index f9e3534df8..e1f0e1e5ff 100644 --- a/libraries/classes/ReplicationGui.php +++ b/libraries/classes/ReplicationGui.php @@ -73,7 +73,7 @@ class ReplicationGui $_url_params['repl_clear_scr'] = true; $html .= '

  • '; $html .= __('Add slave replication user') . '
    • '; } @@ -188,7 +188,7 @@ class ReplicationGui } $_url_params['sr_slave_control_parm'] = 'IO_THREAD'; - $slave_control_io_link = Url::getCommon($_url_params, ''); + $slave_control_io_link = Url::getCommon($_url_params, '', false); if ($server_slave_replication[0]['Slave_SQL_Running'] == 'No') { $_url_params['sr_slave_action'] = 'start'; @@ -197,7 +197,7 @@ class ReplicationGui } $_url_params['sr_slave_control_parm'] = 'SQL_THREAD'; - $slave_control_sql_link = Url::getCommon($_url_params, ''); + $slave_control_sql_link = Url::getCommon($_url_params, '', false); if ($server_slave_replication[0]['Slave_IO_Running'] == 'No' || $server_slave_replication[0]['Slave_SQL_Running'] == 'No' @@ -208,15 +208,15 @@ class ReplicationGui } $_url_params['sr_slave_control_parm'] = null; - $slave_control_full_link = Url::getCommon($_url_params, ''); + $slave_control_full_link = Url::getCommon($_url_params, '', false); $_url_params['sr_slave_action'] = 'reset'; - $slave_control_reset_link = Url::getCommon($_url_params, ''); + $slave_control_reset_link = Url::getCommon($_url_params, '', false); $_url_params = $GLOBALS['url_params']; $_url_params['sr_take_action'] = true; $_url_params['sr_slave_skip_error'] = true; - $slave_skip_error_link = Url::getCommon($_url_params, ''); + $slave_skip_error_link = Url::getCommon($_url_params, '', false); if ($server_slave_replication[0]['Slave_SQL_Running'] == 'No') { $html .= Message::error( @@ -233,7 +233,7 @@ class ReplicationGui $_url_params['sl_configure'] = true; $_url_params['repl_clear_scr'] = true; - $reconfiguremaster_link = Url::getCommon($_url_params, ''); + $reconfiguremaster_link = Url::getCommon($_url_params, '', false); $html .= __( 'Server is configured as slave in a replication process. Would you ' . @@ -293,7 +293,7 @@ class ReplicationGui 'This server is not configured as slave in a replication process. ' . 'Would you like to %sconfigure%s it?' ), - '', + '', '' ); } @@ -355,7 +355,7 @@ class ReplicationGui 'This server is not configured as master in a replication process. ' . 'Would you like to %sconfigure%s it?' ), - '', + '', '' ); $html .= ''; diff --git a/libraries/classes/Server/Privileges.php b/libraries/classes/Server/Privileges.php index a62d111606..eb34b250b3 100644 --- a/libraries/classes/Server/Privileges.php +++ b/libraries/classes/Server/Privileges.php @@ -2844,7 +2844,7 @@ class Privileges $html .= ' href="server_privileges.php'; if ($linktype == 'revoke') { - $html .= '" data-post="' . Url::getCommon($params, ''); + $html .= '" data-post="' . Url::getCommon($params, '', false); } else { $html .= Url::getCommon($params); } diff --git a/libraries/classes/Server/Status/Processes.php b/libraries/classes/Server/Status/Processes.php index 336b28b6ee..df888b1973 100644 --- a/libraries/classes/Server/Status/Processes.php +++ b/libraries/classes/Server/Status/Processes.php @@ -141,7 +141,7 @@ class Processes } $retval .= ''; - $columnUrl = Url::getCommon($column); + $columnUrl = Url::getCommon($column, '', false); $retval .= ''; $retval .= $column['column_name']; @@ -179,7 +179,7 @@ class Processes if (isset($_POST['sort_order'])) { $url_params['sort_order'] = $_POST['sort_order']; } - $retval .= ''; + $retval .= ''; if ($show_full_sql) { $retval .= Util::getImage('s_partialtext', __('Truncate Shown Queries'), ['class' => 'icon_fulltext']); @@ -275,7 +275,7 @@ class Processes $retval = ''; $retval .= '' + . ' data-post="' . Url::getCommon(['kill' => $process['Id']], '', false) . '">' . __('Kill') . ''; $retval .= '' . $process['Id'] . ''; $retval .= '' . htmlspecialchars($process['User']) . ''; diff --git a/libraries/classes/Server/UserGroups.php b/libraries/classes/Server/UserGroups.php index 337cd9a903..35ecac2443 100644 --- a/libraries/classes/Server/UserGroups.php +++ b/libraries/classes/Server/UserGroups.php @@ -115,7 +115,7 @@ class UserGroups array( 'viewUsers' => 1, 'userGroup' => $groupName ), - '' + '', false ) . '">' . Util::getIcon('b_usrlist', __('View users')) @@ -126,7 +126,7 @@ class UserGroups array( 'editUserGroup' => 1, 'userGroup' => $groupName ), - '' + '', false ) . '">' . Util::getIcon('b_edit', __('Edit')) . ''; @@ -137,7 +137,7 @@ class UserGroups array( 'deleteUserGroup' => 1, 'userGroup' => $groupName ), - '' + '', false ) . '">' . Util::getIcon('b_drop', __('Delete')) . ''; diff --git a/libraries/classes/Sql.php b/libraries/classes/Sql.php index f48c85f45b..e7e6619d85 100644 --- a/libraries/classes/Sql.php +++ b/libraries/classes/Sql.php @@ -227,7 +227,7 @@ class Sql . htmlspecialchars($_POST['curr_value']) . '' . '' . __('Browse foreign values') . ''; diff --git a/libraries/classes/Tracking.php b/libraries/classes/Tracking.php index 82c8b25aac..87c8d04ec7 100644 --- a/libraries/classes/Tracking.php +++ b/libraries/classes/Tracking.php @@ -199,20 +199,20 @@ class Tracking $html .= Url::getCommon($url_params + [ 'version' => $version['version'], 'submit_delete_version' => true, - ], ''); + ], '', false); $html .= '">' . $delete . ''; $html .= '' . $report . ''; $html .= '  '; $html .= '' . $structure . ''; $html .= ''; $html .= ''; diff --git a/libraries/classes/Url.php b/libraries/classes/Url.php index cd1f89a4c0..9ff516458f 100644 --- a/libraries/classes/Url.php +++ b/libraries/classes/Url.php @@ -161,14 +161,15 @@ class Url * * @param mixed $params optional, Contains an associative array with url params * @param string $divider optional character to use instead of '?' + * @param bool $encrypt whether to encrypt URL params * * @return string string with URL parameters * @access public */ - public static function getCommon($params = array(), $divider = '?') + public static function getCommon($params = array(), $divider = '?', $encrypt = true) { return htmlspecialchars( - Url::getCommonRaw($params, $divider) + Url::getCommonRaw($params, $divider, $encrypt) ); } @@ -197,11 +198,12 @@ class Url * * @param mixed $params optional, Contains an associative array with url params * @param string $divider optional character to use instead of '?' + * @param bool $encrypt whether to encrypt URL params * * @return string string with URL parameters * @access public */ - public static function getCommonRaw($params = array(), $divider = '?') + public static function getCommonRaw($params = array(), $divider = '?', $encrypt = true) { /** @var Config $PMA_Config */ global $PMA_Config; @@ -219,7 +221,7 @@ class Url $params['lang'] = $GLOBALS['lang']; } - $query = self::buildHttpQuery($params); + $query = self::buildHttpQuery($params, $encrypt); if ($divider != '?' || strlen($query) > 0) { return $divider . $query; @@ -230,15 +232,17 @@ class Url /** * @param array $params + * @param bool $encrypt whether to encrypt URL params + * * @return string */ - public static function buildHttpQuery($params) + public static function buildHttpQuery($params, $encrypt = true) { global $PMA_Config; $separator = self::getArgSeparator(); - if (! $PMA_Config->get('URLQueryEncryption')) { + if (! $encrypt || ! $PMA_Config->get('URLQueryEncryption')) { return http_build_query($params, null, $separator); } @@ -279,6 +283,7 @@ class Url /** * @param string $query + * * @return string */ public static function encryptQuery($query) @@ -290,6 +295,7 @@ class Url /** * @param string $query + * * @return string|null */ public static function decryptQuery($query) diff --git a/setup/frames/index.inc.php b/setup/frames/index.inc.php index f3c369b481..aa4c5a326a 100644 --- a/setup/frames/index.inc.php +++ b/setup/frames/index.inc.php @@ -157,7 +157,7 @@ if ($cf->getServerCount() > 0) { , __('Edit') , ''; echo ' | '; echo ''; + echo '" data-post="' . Url::getCommon(array('token' => $_SESSION[' PMA_token ']), '', false) . '">'; echo __('Delete') . ''; echo ''; echo ''; diff --git a/templates/database/tracking/tracked_tables.twig b/templates/database/tracking/tracked_tables.twig index 9f7a755fc2..f5761e2d19 100644 --- a/templates/database/tracking/tracked_tables.twig +++ b/templates/database/tracking/tracked_tables.twig @@ -50,7 +50,7 @@ 'back': 'db_tracking.php', 'table': version.table_name, 'delete_tracking': true - }, '') }}"> + }, '', false) }}"> {{ Util_getIcon('b_drop', 'Delete tracking'|trans) }} @@ -61,7 +61,7 @@ 'goto': 'tbl_tracking.php', 'back': 'db_tracking.php', 'table': version.table_name - }, '') }}"> + }, '', false) }}"> {{ Util_getIcon('b_versions', 'Versions'|trans) }} + }, '', false) }}"> {{ Util_getIcon('b_report', 'Tracking report'|trans) }} + }, '', false) }}"> {{ Util_getIcon('b_props', 'Structure snapshot'|trans) }} diff --git a/templates/server/databases/databases_footer.twig b/templates/server/databases/databases_footer.twig index 1da193cbef..3efdcbfde4 100644 --- a/templates/server/databases/databases_footer.twig +++ b/templates/server/databases/databases_footer.twig @@ -67,7 +67,7 @@ {{ Message_notice('Note: Enabling the database statistics here might cause heavy traffic between the web server and the MySQL server.'|trans) }}