Skip to content

Commit 5fd8087

Browse files
ip1981cvicentiu
ip1981
authored and
cvicentiu
committed
[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>
1 parent f9b5acf commit 5fd8087

File tree

3 files changed

+183
-7
lines changed

3 files changed

+183
-7
lines changed

‎mysql-test/suite/roles/set_role-9614.result

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
#
2+
# MDEV-9614 Roles and Users Longer than 6 characters
3+
#
4+
# This test case checks the edge case presented in the MDEV. The
5+
# real issue is actually apparent when the username is longer than the
6+
# rolename.
7+
#
8+
# We need a separate database not including test or test_% names. Due to
9+
# default privileges given on these databases.
10+
#
11+
DROP DATABASE IF EXISTS `bug_db`;
12+
Warnings:
13+
Note 1008 Can't drop database 'bug_db'; database doesn't exist
14+
#
15+
# The first user did not show the bug as john's length is smaller
16+
# than client. The bug is apparent most of the time for usertestjohn.
17+
#
18+
CREATE USER `john`@`%`;
19+
CREATE USER `usertestjohn`@`%`;
20+
CREATE ROLE `client`;
21+
#
22+
# Setup the required tables.
23+
#
24+
CREATE DATABASE `bug_db`;
25+
CREATE TABLE `bug_db`.`t0`(`c0` INT);
26+
#
27+
# Setup select privileges only on the role. Setting the role should give
28+
# select access to bug_db.t0.
29+
#
30+
GRANT SELECT ON `bug_db`.`t0` TO `client`;
31+
GRANT `client` TO `john`@`%`;
32+
GRANT `client` TO `usertestjohn`@`%`;
33+
#
34+
# Check to see grants are set.
35+
#
36+
SHOW GRANTS FOR `john`@`%`;
37+
Grants for john@%
38+
GRANT client TO 'john'@'%'
39+
GRANT USAGE ON *.* TO 'john'@'%'
40+
SHOW GRANTS FOR `usertestjohn`@`%`;
41+
Grants for usertestjohn@%
42+
GRANT client TO 'usertestjohn'@'%'
43+
GRANT USAGE ON *.* TO 'usertestjohn'@'%'
44+
SHOW GRANTS FOR `client`;
45+
Grants for client
46+
GRANT USAGE ON *.* TO 'client'
47+
GRANT SELECT ON `bug_db`.`t0` TO 'client'
48+
show databases;
49+
Database
50+
bug_db
51+
information_schema
52+
mtr
53+
mysql
54+
performance_schema
55+
test
56+
#
57+
# Try using the database as john.
58+
#
59+
connect john, localhost, john,,information_schema;
60+
show databases;
61+
Database
62+
information_schema
63+
test
64+
set role client;
65+
show databases;
66+
Database
67+
bug_db
68+
information_schema
69+
test
70+
use bug_db;
71+
#
72+
# Try using the database as usertestjohn.
73+
#
74+
connect usertestjohn, localhost, usertestjohn,,information_schema;
75+
show databases;
76+
Database
77+
information_schema
78+
test
79+
set role client;
80+
show databases;
81+
Database
82+
bug_db
83+
information_schema
84+
test
85+
show grants;
86+
Grants for usertestjohn@%
87+
GRANT client TO 'usertestjohn'@'%'
88+
GRANT USAGE ON *.* TO 'usertestjohn'@'%'
89+
GRANT USAGE ON *.* TO 'client'
90+
GRANT SELECT ON `bug_db`.`t0` TO 'client'
91+
use bug_db;
92+
#
93+
# Cleanup
94+
#
95+
connection default;
96+
drop user john;
97+
drop user usertestjohn;
98+
drop role client;
99+
drop database bug_db;

‎mysql-test/suite/roles/set_role-9614.test

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
--source include/not_embedded.inc
2+
3+
--echo #
4+
--echo # MDEV-9614 Roles and Users Longer than 6 characters
5+
--echo #
6+
--echo # This test case checks the edge case presented in the MDEV. The
7+
--echo # real issue is actually apparent when the username is longer than the
8+
--echo # rolename.
9+
10+
--enable_connect_log
11+
--echo #
12+
--echo # We need a separate database not including test or test_% names. Due to
13+
--echo # default privileges given on these databases.
14+
--echo #
15+
DROP DATABASE IF EXISTS `bug_db`;
16+
17+
--echo #
18+
--echo # The first user did not show the bug as john's length is smaller
19+
--echo # than client. The bug is apparent most of the time for usertestjohn.
20+
--echo #
21+
CREATE USER `john`@`%`;
22+
CREATE USER `usertestjohn`@`%`;
23+
CREATE ROLE `client`;
24+
25+
--echo #
26+
--echo # Setup the required tables.
27+
--echo #
28+
CREATE DATABASE `bug_db`;
29+
CREATE TABLE `bug_db`.`t0`(`c0` INT);
30+
31+
--echo #
32+
--echo # Setup select privileges only on the role. Setting the role should give
33+
--echo # select access to bug_db.t0.
34+
--echo #
35+
GRANT SELECT ON `bug_db`.`t0` TO `client`;
36+
GRANT `client` TO `john`@`%`;
37+
GRANT `client` TO `usertestjohn`@`%`;
38+
39+
--echo #
40+
--echo # Check to see grants are set.
41+
--echo #
42+
SHOW GRANTS FOR `john`@`%`;
43+
SHOW GRANTS FOR `usertestjohn`@`%`;
44+
SHOW GRANTS FOR `client`;
45+
46+
show databases;
47+
48+
--echo #
49+
--echo # Try using the database as john.
50+
--echo #
51+
connect (john, localhost, john,,information_schema);
52+
53+
show databases;
54+
set role client;
55+
show databases;
56+
use bug_db;
57+
58+
--echo #
59+
--echo # Try using the database as usertestjohn.
60+
--echo #
61+
connect (usertestjohn, localhost, usertestjohn,,information_schema);
62+
63+
show databases;
64+
set role client;
65+
show databases;
66+
67+
show grants;
68+
use bug_db;
69+
70+
71+
--echo #
72+
--echo # Cleanup
73+
--echo #
74+
connection default;
75+
drop user john;
76+
drop user usertestjohn;
77+
drop role client;
78+
drop database bug_db;
79+
--disable_connect_log

‎sql/sql_acl.cc

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7195,8 +7195,7 @@ bool check_grant_db(THD *thd, const char *db)
71957195
len= (uint) (end - helping) + 1;
71967196

71977197
/*
7198-
If a role is set, we need to check for privileges
7199-
here aswell
7198+
If a role is set, we need to check for privileges here as well.
72007199
*/
72017200
if (sctx->priv_role[0])
72027201
{
@@ -7210,19 +7209,18 @@ bool check_grant_db(THD *thd, const char *db)
72107209

72117210
for (uint idx=0 ; idx < column_priv_hash.records ; idx++)
72127211
{
7213-
GRANT_TABLE *grant_table= (GRANT_TABLE*)
7214-
my_hash_element(&column_priv_hash,
7215-
idx);
7212+
GRANT_TABLE *grant_table= (GRANT_TABLE*) my_hash_element(&column_priv_hash,
7213+
idx);
72167214
if (len < grant_table->key_length &&
7217-
!memcmp(grant_table->hash_key,helping,len) &&
7215+
!memcmp(grant_table->hash_key, helping, len) &&
72187216
compare_hostname(&grant_table->host, sctx->host, sctx->ip))
72197217
{
72207218
error= FALSE; /* Found match. */
72217219
break;
72227220
}
72237221
if (sctx->priv_role[0] &&
72247222
len2 < grant_table->key_length &&
7225-
!memcmp(grant_table->hash_key,helping2,len) &&
7223+
!memcmp(grant_table->hash_key, helping2, len2) &&
72267224
(!grant_table->host.hostname || !grant_table->host.hostname[0]))
72277225
{
72287226
error= FALSE; /* Found role match */

0 commit comments

Comments
 (0)