From 4bf8bfcaa16dd90d7b36c2c3f5e2d36c7b249bd2 Mon Sep 17 00:00:00 2001 From: William Desportes Date: Wed, 12 Jun 2019 10:43:15 +0200 Subject: [PATCH 1/7] Fix broken foreign key links Fixes: #15225 - Using Command+Click to open in new tab does not work (Firefox/Safari) Fixes: #14270 - Middle-click on foreign key link broken Fixes: #14363 - Broken relational links in tables Signed-off-by: William Desportes --- libraries/classes/Core.php | 24 ++++++++++++++++++++++++ libraries/classes/Display/Results.php | 15 +++++++++------ libraries/classes/Util.php | 3 ++- sql.php | 5 +++++ 4 files changed, 40 insertions(+), 7 deletions(-) diff --git a/libraries/classes/Core.php b/libraries/classes/Core.php index 447b951daa..50e1a58744 100644 --- a/libraries/classes/Core.php +++ b/libraries/classes/Core.php @@ -1289,4 +1289,28 @@ class Core self::fatalError(__('possible exploit')); } } + + /** + * Sign the sql query using hmac using the session token + * + * @param string $sqlQuery The sql query + * @return void + */ + public static function signSqlQuery(string $sqlQuery) + { + return hash_hmac('sha256', $sqlQuery, $_SESSION[' PMA_token ']); + } + + /** + * Check that the sql query has a valid hmac signature + * + * @param string $sqlQuery The sql query + * @return void + */ + public static function checkSqlQuerySignature(string $sqlQuery, string $signature) + { + $hmac = hash_hmac('sha256', $sqlQuery, $_SESSION[' PMA_token ']); + return hash_equals($hmac, $signature); + } + } diff --git a/libraries/classes/Display/Results.php b/libraries/classes/Display/Results.php index 87d0879b5f..cde874aee5 100644 --- a/libraries/classes/Display/Results.php +++ b/libraries/classes/Display/Results.php @@ -5313,16 +5313,19 @@ class Results $title = htmlspecialchars($data); } + $sqlQuery = 'SELECT * FROM ' + . Util::backquote($map[$meta->name][3]) . '.' + . Util::backquote($map[$meta->name][0]) + . ' WHERE ' + . Util::backquote($map[$meta->name][1]) + . $where_comparison; + $_url_params = array( 'db' => $map[$meta->name][3], 'table' => $map[$meta->name][0], 'pos' => '0', - 'sql_query' => 'SELECT * FROM ' - . Util::backquote($map[$meta->name][3]) . '.' - . Util::backquote($map[$meta->name][0]) - . ' WHERE ' - . Util::backquote($map[$meta->name][1]) - . $where_comparison, + 'sql_signature' => Core::signSqlQuery($sqlQuery), + 'sql_query' => $sqlQuery, ); if ($transformation_plugin != $default_function) { diff --git a/libraries/classes/Util.php b/libraries/classes/Util.php index a3f08ad4ec..c64a709444 100644 --- a/libraries/classes/Util.php +++ b/libraries/classes/Util.php @@ -1760,7 +1760,8 @@ class Util $tag_params_strings = array(); if (($url_length > $GLOBALS['cfg']['LinkLengthLimit']) || ! $in_suhosin_limits - || strpos($url, 'sql_query=') !== false + // Has as sql_query without a signature + || ( strpos($url, 'sql_query=') !== false && strpos($url, 'sql_signature=') === false) || strpos($url, 'view[as]=') !== false ) { $parts = explode('?', $url, 2); diff --git a/sql.php b/sql.php index 5e73353519..8a5c2cd755 100644 --- a/sql.php +++ b/sql.php @@ -13,6 +13,7 @@ use PhpMyAdmin\Response; use PhpMyAdmin\Sql; use PhpMyAdmin\Url; use PhpMyAdmin\Util; +use PhpMyAdmin\Core; /** * Gets some core libraries @@ -71,6 +72,10 @@ if (isset($_POST['bkm_fields']['bkm_sql_query'])) { $sql_query = $_POST['bkm_fields']['bkm_sql_query']; } elseif (isset($_POST['sql_query'])) { $sql_query = $_POST['sql_query']; +} elseif (isset($_GET['sql_query']) && isset($_GET['sql_signature'])) { + if (Core::checkSqlQuerySignature($_GET['sql_query'], $_GET['sql_signature'])) { + $sql_query = $_GET['sql_query']; + } } // This one is just to fill $db From b61d878ac4fbc74a669dfcfa081e62e6ad88b1ec Mon Sep 17 00:00:00 2001 From: William Desportes Date: Wed, 12 Jun 2019 13:31:24 +0200 Subject: [PATCH 2/7] Add unit tests Signed-off-by: William Desportes --- test/classes/CoreTest.php | 66 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/test/classes/CoreTest.php b/test/classes/CoreTest.php index ddda7b7010..43a504094c 100644 --- a/test/classes/CoreTest.php +++ b/test/classes/CoreTest.php @@ -1121,4 +1121,70 @@ class CoreTest extends PmaTestCase $this->assertGreaterThan(0, mb_strpos($printed, $warn)); } + + /** + * Test for Core::signSqlQuery + * + * @return void + */ + function testSignSqlQuery() + { + $_SESSION[' PMA_token '] = hash('sha1', 'test'); + $sqlQuery = 'SELECT * FROM `test`.`db` WHERE 1;'; + $signature = Core::signSqlQuery($sqlQuery); + $hmac = '33371e8680a640dc05944a2a24e6e630d3e9e3dba24464135f2fb954c3a4ffe2'; + $this->assertSame($hmac, $signature, 'The signature must match the computed one'); + } + + /** + * Test for Core::checkSqlQuerySignature + * + * @return void + */ + function testCheckSqlQuerySignature() + { + $_SESSION[' PMA_token '] = hash('sha1', 'test'); + $sqlQuery = 'SELECT * FROM `test`.`db` WHERE 1;'; + $hmac = '33371e8680a640dc05944a2a24e6e630d3e9e3dba24464135f2fb954c3a4ffe2'; + $this->assertTrue(Core::checkSqlQuerySignature($sqlQuery, $hmac)); + } + + /** + * Test for Core::checkSqlQuerySignature + * + * @return void + */ + function testCheckSqlQuerySignatureFails() + { + $_SESSION[' PMA_token '] = hash('sha1', '132654987gguieunofz'); + $sqlQuery = 'SELECT * FROM `test`.`db` WHERE 1;'; + $hmac = '33371e8680a640dc05944a2a24e6e630d3e9e3dba24464135f2fb954c3a4ffe2'; + $this->assertFalse(Core::checkSqlQuerySignature($sqlQuery, $hmac)); + } + + /** + * Test for Core::checkSqlQuerySignature + * + * @return void + */ + function testCheckSqlQuerySignatureFailsBadHash() + { + $_SESSION[' PMA_token '] = hash('sha1', 'test'); + $sqlQuery = 'SELECT * FROM `test`.`db` WHERE 1;'; + $hmac = '3333333380a640dc05944a2a24e6e630d3e9e3dba24464135f2fb954c3eeeeee'; + $this->assertFalse(Core::checkSqlQuerySignature($sqlQuery, $hmac)); + } + + /** + * Test for Core::checkSqlQuerySignature + * + * @return void + */ + function testCheckSqlQuerySignatureFailsNoSession() + { + $_SESSION[' PMA_token '] = null; + $sqlQuery = 'SELECT * FROM `test`.`db` WHERE 1;'; + $hmac = '3333333380a640dc05944a2a24e6e630d3e9e3dba24464135f2fb954c3eeeeee'; + $this->assertFalse(Core::checkSqlQuerySignature($sqlQuery, $hmac)); + } } From c59790b05ec193b99f7de2c115d7014086912b33 Mon Sep 17 00:00:00 2001 From: William Desportes Date: Wed, 12 Jun 2019 13:36:58 +0200 Subject: [PATCH 3/7] Remove typehints for hhvm Signed-off-by: William Desportes --- libraries/classes/Core.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libraries/classes/Core.php b/libraries/classes/Core.php index 50e1a58744..2428f569bf 100644 --- a/libraries/classes/Core.php +++ b/libraries/classes/Core.php @@ -1296,7 +1296,7 @@ class Core * @param string $sqlQuery The sql query * @return void */ - public static function signSqlQuery(string $sqlQuery) + public static function signSqlQuery($sqlQuery) { return hash_hmac('sha256', $sqlQuery, $_SESSION[' PMA_token ']); } @@ -1307,7 +1307,7 @@ class Core * @param string $sqlQuery The sql query * @return void */ - public static function checkSqlQuerySignature(string $sqlQuery, string $signature) + public static function checkSqlQuerySignature($sqlQuery, $signature) { $hmac = hash_hmac('sha256', $sqlQuery, $_SESSION[' PMA_token ']); return hash_equals($hmac, $signature); From 80d88d2f1829f81fdc0d23f8d211378195525bce Mon Sep 17 00:00:00 2001 From: William Desportes Date: Wed, 12 Jun 2019 13:47:20 +0200 Subject: [PATCH 4/7] Add a key for hhvm Signed-off-by: William Desportes --- test/classes/CoreTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/classes/CoreTest.php b/test/classes/CoreTest.php index 43a504094c..615a50dc3a 100644 --- a/test/classes/CoreTest.php +++ b/test/classes/CoreTest.php @@ -1182,7 +1182,7 @@ class CoreTest extends PmaTestCase */ function testCheckSqlQuerySignatureFailsNoSession() { - $_SESSION[' PMA_token '] = null; + $_SESSION[' PMA_token '] = 'empty'; $sqlQuery = 'SELECT * FROM `test`.`db` WHERE 1;'; $hmac = '3333333380a640dc05944a2a24e6e630d3e9e3dba24464135f2fb954c3eeeeee'; $this->assertFalse(Core::checkSqlQuerySignature($sqlQuery, $hmac)); From fb4a8d61b7cb9e7057dd6b5684d1c3b501a9026b Mon Sep 17 00:00:00 2001 From: William Desportes Date: Thu, 13 Jun 2019 07:43:48 +0200 Subject: [PATCH 5/7] Add Unit test case for session renewal Signed-off-by: William Desportes --- test/classes/CoreTest.php | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/test/classes/CoreTest.php b/test/classes/CoreTest.php index 615a50dc3a..85ff18faf1 100644 --- a/test/classes/CoreTest.php +++ b/test/classes/CoreTest.php @@ -1187,4 +1187,20 @@ class CoreTest extends PmaTestCase $hmac = '3333333380a640dc05944a2a24e6e630d3e9e3dba24464135f2fb954c3eeeeee'; $this->assertFalse(Core::checkSqlQuerySignature($sqlQuery, $hmac)); } + + /** + * Test for Core::checkSqlQuerySignature + * + * @return void + */ + function testCheckSqlQuerySignatureFailsFromAnotherSession() + { + $_SESSION[' PMA_token '] = hash('sha1', 'firstSession'); + $sqlQuery = 'SELECT * FROM `test`.`db` WHERE 1;'; + $hmac = Core::signSqlQuery($sqlQuery); + $this->assertTrue(Core::checkSqlQuerySignature($sqlQuery, $hmac)); + $_SESSION[' PMA_token '] = hash('sha1', 'secondSession'); + // Try to use the token (hmac) from the previous session + $this->assertFalse(Core::checkSqlQuerySignature($sqlQuery, $hmac)); + } } From 80a7f0a75c72ec2b92216647ac66890ff58002f8 Mon Sep 17 00:00:00 2001 From: William Desportes Date: Thu, 13 Jun 2019 19:04:57 +0200 Subject: [PATCH 6/7] Make the hmac secret a secret for the user Signed-off-by: William Desportes --- libraries/classes/Core.php | 4 ++-- .../classes/Plugins/Auth/AuthenticationSignon.php | 1 + libraries/classes/Session.php | 1 + test/classes/CoreTest.php | 14 +++++++------- 4 files changed, 11 insertions(+), 9 deletions(-) diff --git a/libraries/classes/Core.php b/libraries/classes/Core.php index 2428f569bf..40ae15a221 100644 --- a/libraries/classes/Core.php +++ b/libraries/classes/Core.php @@ -1298,7 +1298,7 @@ class Core */ public static function signSqlQuery($sqlQuery) { - return hash_hmac('sha256', $sqlQuery, $_SESSION[' PMA_token ']); + return hash_hmac('sha256', $sqlQuery, $_SESSION[' HMAC_secret ']); } /** @@ -1309,7 +1309,7 @@ class Core */ public static function checkSqlQuerySignature($sqlQuery, $signature) { - $hmac = hash_hmac('sha256', $sqlQuery, $_SESSION[' PMA_token ']); + $hmac = hash_hmac('sha256', $sqlQuery, $_SESSION[' HMAC_secret ']); return hash_equals($hmac, $signature); } diff --git a/libraries/classes/Plugins/Auth/AuthenticationSignon.php b/libraries/classes/Plugins/Auth/AuthenticationSignon.php index 828727eac5..a6d1c7bf15 100644 --- a/libraries/classes/Plugins/Auth/AuthenticationSignon.php +++ b/libraries/classes/Plugins/Auth/AuthenticationSignon.php @@ -214,6 +214,7 @@ class AuthenticationSignon extends AuthenticationPlugin /* Restore our token */ if (!empty($pma_token)) { $_SESSION[' PMA_token '] = $pma_token; + $_SESSION[' HMAC_secret '] = Util::generateRandom(16); } /** diff --git a/libraries/classes/Session.php b/libraries/classes/Session.php index 45799d4bb9..d05bbc7a41 100644 --- a/libraries/classes/Session.php +++ b/libraries/classes/Session.php @@ -29,6 +29,7 @@ class Session private static function generateToken() { $_SESSION[' PMA_token '] = Util::generateRandom(16); + $_SESSION[' HMAC_secret '] = Util::generateRandom(16); /** * Check if token is properly generated (the generation can fail, for example diff --git a/test/classes/CoreTest.php b/test/classes/CoreTest.php index 85ff18faf1..7f508f525a 100644 --- a/test/classes/CoreTest.php +++ b/test/classes/CoreTest.php @@ -1129,7 +1129,7 @@ class CoreTest extends PmaTestCase */ function testSignSqlQuery() { - $_SESSION[' PMA_token '] = hash('sha1', 'test'); + $_SESSION[' HMAC_secret '] = hash('sha1', 'test'); $sqlQuery = 'SELECT * FROM `test`.`db` WHERE 1;'; $signature = Core::signSqlQuery($sqlQuery); $hmac = '33371e8680a640dc05944a2a24e6e630d3e9e3dba24464135f2fb954c3a4ffe2'; @@ -1143,7 +1143,7 @@ class CoreTest extends PmaTestCase */ function testCheckSqlQuerySignature() { - $_SESSION[' PMA_token '] = hash('sha1', 'test'); + $_SESSION[' HMAC_secret '] = hash('sha1', 'test'); $sqlQuery = 'SELECT * FROM `test`.`db` WHERE 1;'; $hmac = '33371e8680a640dc05944a2a24e6e630d3e9e3dba24464135f2fb954c3a4ffe2'; $this->assertTrue(Core::checkSqlQuerySignature($sqlQuery, $hmac)); @@ -1156,7 +1156,7 @@ class CoreTest extends PmaTestCase */ function testCheckSqlQuerySignatureFails() { - $_SESSION[' PMA_token '] = hash('sha1', '132654987gguieunofz'); + $_SESSION[' HMAC_secret '] = hash('sha1', '132654987gguieunofz'); $sqlQuery = 'SELECT * FROM `test`.`db` WHERE 1;'; $hmac = '33371e8680a640dc05944a2a24e6e630d3e9e3dba24464135f2fb954c3a4ffe2'; $this->assertFalse(Core::checkSqlQuerySignature($sqlQuery, $hmac)); @@ -1169,7 +1169,7 @@ class CoreTest extends PmaTestCase */ function testCheckSqlQuerySignatureFailsBadHash() { - $_SESSION[' PMA_token '] = hash('sha1', 'test'); + $_SESSION[' HMAC_secret '] = hash('sha1', 'test'); $sqlQuery = 'SELECT * FROM `test`.`db` WHERE 1;'; $hmac = '3333333380a640dc05944a2a24e6e630d3e9e3dba24464135f2fb954c3eeeeee'; $this->assertFalse(Core::checkSqlQuerySignature($sqlQuery, $hmac)); @@ -1182,7 +1182,7 @@ class CoreTest extends PmaTestCase */ function testCheckSqlQuerySignatureFailsNoSession() { - $_SESSION[' PMA_token '] = 'empty'; + $_SESSION[' HMAC_secret '] = 'empty'; $sqlQuery = 'SELECT * FROM `test`.`db` WHERE 1;'; $hmac = '3333333380a640dc05944a2a24e6e630d3e9e3dba24464135f2fb954c3eeeeee'; $this->assertFalse(Core::checkSqlQuerySignature($sqlQuery, $hmac)); @@ -1195,11 +1195,11 @@ class CoreTest extends PmaTestCase */ function testCheckSqlQuerySignatureFailsFromAnotherSession() { - $_SESSION[' PMA_token '] = hash('sha1', 'firstSession'); + $_SESSION[' HMAC_secret '] = hash('sha1', 'firstSession'); $sqlQuery = 'SELECT * FROM `test`.`db` WHERE 1;'; $hmac = Core::signSqlQuery($sqlQuery); $this->assertTrue(Core::checkSqlQuerySignature($sqlQuery, $hmac)); - $_SESSION[' PMA_token '] = hash('sha1', 'secondSession'); + $_SESSION[' HMAC_secret '] = hash('sha1', 'secondSession'); // Try to use the token (hmac) from the previous session $this->assertFalse(Core::checkSqlQuerySignature($sqlQuery, $hmac)); } From fc014c5b727d45508f248313dd241ba9ae1a84cb Mon Sep 17 00:00:00 2001 From: William Desportes Date: Thu, 13 Jun 2019 20:04:54 +0200 Subject: [PATCH 7/7] Harden the HMAC secret by using the blowfish_secret Signed-off-by: William Desportes --- libraries/classes/Core.php | 8 ++++++-- test/classes/CoreTest.php | 24 ++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/libraries/classes/Core.php b/libraries/classes/Core.php index 40ae15a221..be8b21ad86 100644 --- a/libraries/classes/Core.php +++ b/libraries/classes/Core.php @@ -1298,7 +1298,9 @@ class Core */ public static function signSqlQuery($sqlQuery) { - return hash_hmac('sha256', $sqlQuery, $_SESSION[' HMAC_secret ']); + /** @var array $cfg */ + global $cfg; + return hash_hmac('sha256', $sqlQuery, $_SESSION[' HMAC_secret '] . $cfg['blowfish_secret']); } /** @@ -1309,7 +1311,9 @@ class Core */ public static function checkSqlQuerySignature($sqlQuery, $signature) { - $hmac = hash_hmac('sha256', $sqlQuery, $_SESSION[' HMAC_secret ']); + /** @var array $cfg */ + global $cfg; + $hmac = hash_hmac('sha256', $sqlQuery, $_SESSION[' HMAC_secret '] . $cfg['blowfish_secret']); return hash_equals($hmac, $signature); } diff --git a/test/classes/CoreTest.php b/test/classes/CoreTest.php index 7f508f525a..26b1de9a88 100644 --- a/test/classes/CoreTest.php +++ b/test/classes/CoreTest.php @@ -1203,4 +1203,28 @@ class CoreTest extends PmaTestCase // Try to use the token (hmac) from the previous session $this->assertFalse(Core::checkSqlQuerySignature($sqlQuery, $hmac)); } + + /** + * Test for Core::checkSqlQuerySignature + * + * @return void + */ + function testCheckSqlQuerySignatureFailsBlowfishSecretChanged() + { + $GLOBALS['cfg']['blowfish_secret'] = ''; + $_SESSION[' HMAC_secret '] = hash('sha1', 'firstSession'); + $sqlQuery = 'SELECT * FROM `test`.`db` WHERE 1;'; + $hmac = Core::signSqlQuery($sqlQuery); + $this->assertTrue(Core::checkSqlQuerySignature($sqlQuery, $hmac)); + $GLOBALS['cfg']['blowfish_secret'] = '32154987zd'; + // Try to use the previous HMAC signature + $this->assertFalse(Core::checkSqlQuerySignature($sqlQuery, $hmac)); + + $GLOBALS['cfg']['blowfish_secret'] = '32154987zd'; + // Generate the HMAC signature to check that it works + $hmac = Core::signSqlQuery($sqlQuery); + // Must work now, (good secret and blowfish_secret) + $this->assertTrue(Core::checkSqlQuerySignature($sqlQuery, $hmac)); + } + }