From 3c58ae9e74d306b09ed6de6c82c9b4079f199169 Mon Sep 17 00:00:00 2001 From: "dlenev@brandersnatch.localdomain" <> Date: Mon, 1 Mar 2004 09:15:33 +0300 Subject: [PATCH 1/2] Fixed small race condition, when global query_id was modified without proper locking. --- BitKeeper/etc/logging_ok | 1 + sql/sp_head.cc | 3 +++ sql/sql_parse.cc | 4 ++++ 3 files changed, 8 insertions(+) diff --git a/BitKeeper/etc/logging_ok b/BitKeeper/etc/logging_ok index 6e53347b0a6..b8607af3b3c 100644 --- a/BitKeeper/etc/logging_ok +++ b/BitKeeper/etc/logging_ok @@ -24,6 +24,7 @@ bk@admin.bk bk@mysql.r18.ru carsten@tsort.bitbybit.dk davida@isil.mysql.com +dlenev@brandersnatch.localdomain dlenev@build.mysql.com dlenev@mysql.com gerberb@ou800.zenez.com diff --git a/sql/sp_head.cc b/sql/sp_head.cc index a68118cecd1..bfd7ad259f6 100644 --- a/sql/sp_head.cc +++ b/sql/sp_head.cc @@ -796,7 +796,10 @@ sp_instr_stmt::exec_stmt(THD *thd, LEX *lex) thd->lex->unit.thd= thd; // QQ Not reentrant freelist= thd->free_list; thd->free_list= NULL; + + VOID(pthread_mutex_lock(&LOCK_thread_count)); thd->query_id= query_id++; + VOID(pthread_mutex_unlock(&LOCK_thread_count)); // Copy WHERE clause pointers to avoid damaging by optimisation // Also clear ref_pointer_arrays. diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index ae507c34272..d332b6985cb 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -1086,6 +1086,10 @@ extern "C" pthread_handler_decl(handle_bootstrap,arg) thd->query_length=length; thd->query= thd->memdup_w_gap(buff, length+1, thd->db_length+1); thd->query[length] = '\0'; + /* + We don't need to obtain LOCK_thread_count here because in bootstrap + mode we have only one thread. + */ thd->query_id=query_id++; if (mqh_used && thd->user_connect && check_mqh(thd, SQLCOM_END)) { From 1631ba625f9db24372b1f02b73fa0d5ffcab85fd Mon Sep 17 00:00:00 2001 From: "pem@mysql.comhem.se" <> Date: Tue, 2 Mar 2004 11:52:19 +0100 Subject: [PATCH 2/2] Fixed BUG#2777: Stored procedure doesn't observe definer's rights. SQL SECURITY DEFINER must enforce reduced rights too, not just additional rights. --- mysql-test/r/sp-security.result | 49 +++++++++++++++++++--- mysql-test/t/sp-security.test | 73 +++++++++++++++++++++++++++++---- sql/sql_acl.cc | 22 +++++++++- 3 files changed, 129 insertions(+), 15 deletions(-) diff --git a/mysql-test/r/sp-security.result b/mysql-test/r/sp-security.result index 9d5f71225b6..c4fbece9d72 100644 --- a/mysql-test/r/sp-security.result +++ b/mysql-test/r/sp-security.result @@ -1,5 +1,6 @@ use test; -grant usage on *.* to dummy@localhost; +grant usage on *.* to user1@localhost; +flush privileges; drop database if exists db1_secret; create database db1_secret; use db1_secret; @@ -15,14 +16,14 @@ u i root@localhost 1 call stamp(2); select * from db1_secret.t1; -ERROR 42000: Access denied for user: 'dummy'@'localhost' to database 'db1_secret' +ERROR 42000: Access denied for user: 'user1'@'localhost' to database 'db1_secret' call stamp(3); select * from db1_secret.t1; ERROR 42000: Access denied for user: ''@'localhost' to database 'db1_secret' select * from t1; u i root@localhost 1 -dummy@localhost 2 +user1@localhost 2 anon@localhost 3 alter procedure stamp sql security invoker; show procedure status like 'stamp'; @@ -32,14 +33,50 @@ call stamp(4); select * from t1; u i root@localhost 1 -dummy@localhost 2 +user1@localhost 2 anon@localhost 3 root@localhost 4 call stamp(5); -ERROR 42000: Access denied for user: 'dummy'@'localhost' to database 'db1_secret' +ERROR 42000: Access denied for user: 'user1'@'localhost' to database 'db1_secret' call stamp(6); ERROR 42000: Access denied for user: ''@'localhost' to database 'db1_secret' +drop database if exists db2; +create database db2; +use db2; +create table t2 (s1 int); +insert into t2 values (0); +grant usage on db2.* to user1@localhost; +grant select on db2.* to user1@localhost; +grant usage on db2.* to user2@localhost; +grant select,insert,update,delete on db2.* to user2@localhost; +flush privileges; +use db2; +create procedure p () insert into t2 values (1); +call p(); +ERROR 42000: Access denied for user: 'user1'@'localhost' to database 'db2' +use db2; +call p(); +ERROR 42000: Access denied for user: 'user1'@'localhost' to database 'db2' +select * from t2; +s1 +0 +create procedure q () insert into t2 values (2); +call q(); +select * from t2; +s1 +0 +2 +use db2; +call q(); +select * from t2; +s1 +0 +2 +2 drop procedure stamp; +drop procedure p; +drop procedure q; use test; drop database db1_secret; -delete from mysql.user where user='dummy'; +drop database db2; +delete from mysql.user where user='user1' or user='user2'; diff --git a/mysql-test/t/sp-security.test b/mysql-test/t/sp-security.test index 0d77b53210e..ac7477869a1 100644 --- a/mysql-test/t/sp-security.test +++ b/mysql-test/t/sp-security.test @@ -7,8 +7,9 @@ connect (con1root,localhost,root,,); connection con1root; use test; -# Create dummy user with no particular access rights -grant usage on *.* to dummy@localhost; +# Create user user1 with no particular access rights +grant usage on *.* to user1@localhost; +flush privileges; --disable_warnings drop database if exists db1_secret; @@ -30,13 +31,13 @@ show procedure status like 'stamp'; call stamp(1); select * from t1; -connect (con2dummy,localhost,dummy,,); +connect (con2user1,localhost,user1,,); connect (con3anon,localhost,anon,,); # -# Dummy can +# User1 can # -connection con2dummy; +connection con2user1; # This should work... call stamp(2); @@ -75,9 +76,9 @@ call stamp(4); select * from t1; # -# Dummy cannot +# User1 cannot # -connection con2dummy; +connection con2user1; # This should not work --error 1044 @@ -92,9 +93,65 @@ connection con3anon; --error 1044 call stamp(6); + +# +# BUG#2777 +# + +connection con1root; +--disable_warnings +drop database if exists db2; +--enable_warnings +create database db2; + +use db2; + +create table t2 (s1 int); +insert into t2 values (0); + +grant usage on db2.* to user1@localhost; +grant select on db2.* to user1@localhost; +grant usage on db2.* to user2@localhost; +grant select,insert,update,delete on db2.* to user2@localhost; +flush privileges; + +connection con2user1; +use db2; + +create procedure p () insert into t2 values (1); + +# Check that this doesn't work. +--error 1044 +call p(); + +connect (con4user2,localhost,user2,,); + +connection con4user2; +use db2; + +# This should not work, since p is executed with definer's (user1's) rights. +--error 1044 +call p(); +select * from t2; + +create procedure q () insert into t2 values (2); + +call q(); +select * from t2; + +connection con2user1; +use db2; + +# This should work +call q(); +select * from t2; + # Clean up connection con1root; drop procedure stamp; +drop procedure p; +drop procedure q; use test; drop database db1_secret; -delete from mysql.user where user='dummy'; +drop database db2; +delete from mysql.user where user='user1' or user='user2'; diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc index 5febb49e110..d294055ff8a 100644 --- a/sql/sql_acl.cc +++ b/sql/sql_acl.cc @@ -794,6 +794,7 @@ int acl_getroot_no_password(THD *thd) { ulong user_access= NO_ACCESS; int res= 1; + uint i; ACL_USER *acl_user= 0; DBUG_ENTER("acl_getroot_no_password"); @@ -810,13 +811,16 @@ int acl_getroot_no_password(THD *thd) VOID(pthread_mutex_lock(&acl_cache->lock)); + thd->master_access= 0; + thd->db_access= 0; + /* Find acl entry in user database. This is specially tailored to suit the check we do for CALL of a stored procedure; thd->user is set to what is actually a priv_user, which can be ''. */ - for (uint i=0 ; i < acl_users.elements ; i++) + for (i=0 ; i < acl_users.elements ; i++) { acl_user= dynamic_element(&acl_users,i,ACL_USER*); if ((!acl_user->user && (!thd->user || !thd->user[0])) || @@ -832,6 +836,22 @@ int acl_getroot_no_password(THD *thd) if (acl_user) { + for (i=0 ; i < acl_dbs.elements ; i++) + { + ACL_DB *acl_db= dynamic_element(&acl_dbs, i, ACL_DB*); + if (!acl_db->user || + (thd->user && thd->user[0] && !strcmp(thd->user, acl_db->user))) + { + if (compare_hostname(&acl_db->host, thd->host, thd->ip)) + { + if (!acl_db->db || (thd->db && !strcmp(acl_db->db, thd->db))) + { + thd->db_access= acl_db->access; + break; + } + } + } + } thd->master_access= acl_user->access; thd->priv_user= acl_user->user ? thd->user : (char *) "";