[Commits] eaf5376: [MDEV-9614] Roles and Users longer than 6 characters

Vicentiu Ciorbaru vicentiu at mariadb.org
Mon May 30 21:59:57 EEST 2016


revision-id: eaf5376f808b3d71c442fb6b95048c4dcd684b08 (mariadb-10.0.25-5-geaf5376)
parent(s): f9d453e893c04a648736d46f0059db98b56ab14c
author: Igor Pashev
committer: Vicențiu Ciorbaru
timestamp: 2016-05-30 21:45:23 +0300
message:

[MDEV-9614] Roles and Users longer than 6 characters

The bug is apparent when the username is longer than the rolename.
It is caused by a simple typo that caused a memcmp call to compare a
different number of bytes than necessary.

The fix was proposed by Igor Pashev. I have reviewed it and it is the
correct approach. Test case introduced by me, using the details provided
in the MDEV.

Signed-off-by: Vicențiu Ciorbaru <vicentiu at mariadb.org>

---
 mysql-test/suite/roles/set_role-9614.result | 99 +++++++++++++++++++++++++++++
 mysql-test/suite/roles/set_role-9614.test   | 79 +++++++++++++++++++++++
 sql/sql_acl.cc                              | 12 ++--
 3 files changed, 183 insertions(+), 7 deletions(-)

diff --git a/mysql-test/suite/roles/set_role-9614.result b/mysql-test/suite/roles/set_role-9614.result
new file mode 100644
index 0000000..37f6db0
--- /dev/null
+++ b/mysql-test/suite/roles/set_role-9614.result
@@ -0,0 +1,99 @@
+#
+# MDEV-9614 Roles and Users Longer than 6 characters
+#
+# This test case checks the edge case presented in the MDEV. The
+# real issue is actually apparent when the username is longer than the
+# rolename.
+#
+# We need a separate database not including test or test_% names. Due to
+# default privileges given on these databases.
+#
+DROP DATABASE IF EXISTS `bug_db`;
+Warnings:
+Note	1008	Can't drop database 'bug_db'; database doesn't exist
+#
+# The first user did not show the bug as john's length is smaller
+# than client. The bug is apparent most of the time for usertestjohn.
+#
+CREATE USER `john`@`%`;
+CREATE USER `usertestjohn`@`%`;
+CREATE ROLE `client`;
+#
+# Setup the required tables.
+#
+CREATE DATABASE `bug_db`;
+CREATE TABLE `bug_db`.`t0`(`c0` INT);
+#
+# Setup select privileges only on the role. Setting the role should give
+# select access to bug_db.t0.
+#
+GRANT SELECT ON `bug_db`.`t0` TO `client`;
+GRANT `client` TO `john`@`%`;
+GRANT `client` TO `usertestjohn`@`%`;
+#
+# Check to see grants are set.
+#
+SHOW GRANTS FOR `john`@`%`;
+Grants for john@%
+GRANT client TO 'john'@'%'
+GRANT USAGE ON *.* TO 'john'@'%'
+SHOW GRANTS FOR `usertestjohn`@`%`;
+Grants for usertestjohn@%
+GRANT client TO 'usertestjohn'@'%'
+GRANT USAGE ON *.* TO 'usertestjohn'@'%'
+SHOW GRANTS FOR `client`;
+Grants for client
+GRANT USAGE ON *.* TO 'client'
+GRANT SELECT ON `bug_db`.`t0` TO 'client'
+show databases;
+Database
+bug_db
+information_schema
+mtr
+mysql
+performance_schema
+test
+#
+# Try using the database as john.
+#
+connect  john, localhost, john,,information_schema;
+show databases;
+Database
+information_schema
+test
+set role client;
+show databases;
+Database
+bug_db
+information_schema
+test
+use bug_db;
+#
+# Try using the database as usertestjohn.
+#
+connect  usertestjohn, localhost, usertestjohn,,information_schema;
+show databases;
+Database
+information_schema
+test
+set role client;
+show databases;
+Database
+bug_db
+information_schema
+test
+show grants;
+Grants for usertestjohn@%
+GRANT client TO 'usertestjohn'@'%'
+GRANT USAGE ON *.* TO 'usertestjohn'@'%'
+GRANT USAGE ON *.* TO 'client'
+GRANT SELECT ON `bug_db`.`t0` TO 'client'
+use bug_db;
+#
+# Cleanup
+#
+connection default;
+drop user john;
+drop user usertestjohn;
+drop role client;
+drop database bug_db;
diff --git a/mysql-test/suite/roles/set_role-9614.test b/mysql-test/suite/roles/set_role-9614.test
new file mode 100644
index 0000000..5e9f7da
--- /dev/null
+++ b/mysql-test/suite/roles/set_role-9614.test
@@ -0,0 +1,79 @@
+--source include/not_embedded.inc
+
+--echo #
+--echo # MDEV-9614 Roles and Users Longer than 6 characters
+--echo #
+--echo # This test case checks the edge case presented in the MDEV. The
+--echo # real issue is actually apparent when the username is longer than the
+--echo # rolename.
+
+--enable_connect_log
+--echo #
+--echo # We need a separate database not including test or test_% names. Due to
+--echo # default privileges given on these databases.
+--echo #
+DROP DATABASE IF EXISTS `bug_db`;
+
+--echo #
+--echo # The first user did not show the bug as john's length is smaller
+--echo # than client. The bug is apparent most of the time for usertestjohn.
+--echo #
+CREATE USER `john`@`%`;
+CREATE USER `usertestjohn`@`%`;
+CREATE ROLE `client`;
+
+--echo #
+--echo # Setup the required tables.
+--echo #
+CREATE DATABASE `bug_db`;
+CREATE TABLE `bug_db`.`t0`(`c0` INT);
+
+--echo #
+--echo # Setup select privileges only on the role. Setting the role should give
+--echo # select access to bug_db.t0.
+--echo #
+GRANT SELECT ON `bug_db`.`t0` TO `client`;
+GRANT `client` TO `john`@`%`;
+GRANT `client` TO `usertestjohn`@`%`;
+
+--echo #
+--echo # Check to see grants are set.
+--echo #
+SHOW GRANTS FOR `john`@`%`;
+SHOW GRANTS FOR `usertestjohn`@`%`;
+SHOW GRANTS FOR `client`;
+
+show databases;
+
+--echo #
+--echo # Try using the database as john.
+--echo #
+connect (john, localhost, john,,information_schema);
+
+show databases;
+set role client;
+show databases;
+use bug_db;
+
+--echo #
+--echo # Try using the database as usertestjohn.
+--echo #
+connect (usertestjohn, localhost, usertestjohn,,information_schema);
+
+show databases;
+set role client;
+show databases;
+
+show grants;
+use bug_db;
+
+
+--echo #
+--echo # Cleanup
+--echo #
+connection default;
+drop user john;
+drop user usertestjohn;
+drop role client;
+drop database bug_db;
+--disable_connect_log
diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc
index 8cc9469..f06e9ce 100644
--- a/sql/sql_acl.cc
+++ b/sql/sql_acl.cc
@@ -7195,8 +7195,7 @@ bool check_grant_db(THD *thd, const char *db)
   len= (uint) (end - helping) + 1;
 
   /*
-     If a role is set, we need to check for privileges
-     here aswell
+     If a role is set, we need to check for privileges here as well.
   */
   if (sctx->priv_role[0])
   {
@@ -7210,11 +7209,10 @@ bool check_grant_db(THD *thd, const char *db)
 
   for (uint idx=0 ; idx < column_priv_hash.records ; idx++)
   {
-    GRANT_TABLE *grant_table= (GRANT_TABLE*)
-      my_hash_element(&column_priv_hash,
-                      idx);
+    GRANT_TABLE *grant_table= (GRANT_TABLE*) my_hash_element(&column_priv_hash,
+                                                             idx);
     if (len < grant_table->key_length &&
-	!memcmp(grant_table->hash_key,helping,len) &&
+        !memcmp(grant_table->hash_key, helping, len) &&
         compare_hostname(&grant_table->host, sctx->host, sctx->ip))
     {
       error= FALSE; /* Found match. */
@@ -7222,7 +7220,7 @@ bool check_grant_db(THD *thd, const char *db)
     }
     if (sctx->priv_role[0] &&
         len2 < grant_table->key_length &&
-        !memcmp(grant_table->hash_key,helping2,len) &&
+        !memcmp(grant_table->hash_key, helping2, len2) &&
         (!grant_table->host.hostname || !grant_table->host.hostname[0]))
     {
       error= FALSE; /* Found role match */


More information about the commits mailing list