summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorIgor Pashev <pashev.igor@gmail.com>2016-05-30 21:42:36 +0300
committerVicențiu Ciorbaru <vicentiu@mariadb.org>2016-06-22 16:41:38 +0300
commit5fd80875909c88e624a79a528eaaf9418089a211 (patch)
treeafd33a7875e89093c3e0dda75d16c04c310b87c5
parentf9b5acfb0cdf175e3f2ab555cf082b1ef4b920a9 (diff)
downloadmariadb-git-5fd80875909c88e624a79a528eaaf9418089a211.tar.gz
[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@mariadb.org>
-rw-r--r--mysql-test/suite/roles/set_role-9614.result99
-rw-r--r--mysql-test/suite/roles/set_role-9614.test79
-rw-r--r--sql/sql_acl.cc12
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 00000000000..37f6db070c0
--- /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 00000000000..5e9f7dacf19
--- /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 8cc9469cd09..f06e9ce647c 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 */