List:Commits« Previous MessageNext Message »
From:Dmitry Lenev Date:October 7 2010 4:01pm
Subject:bzr commit into mysql-5.5-runtime branch (Dmitry.Lenev:3159) Bug#57061
View as plain text  
#At file:///home/dlenev/src/bzr/mysql-5.5-rt-57061/ based on revid:jon.hauglid@stripped

 3159 Dmitry Lenev	2010-10-07
      Fix for bug#57061 "User without privilege on routine can 
      discover its existence".
      
      The problem was that user without any privileges on 
      routine was able to find out whether it existed or not.
      DROP FUNCTION and DROP PROCEDURE statements were 
      checking if routine being dropped existed and reported 
      ER_SP_DOES_NOT_EXIST error/warning before checking 
      if user had enough privileges to drop it.
      
      This patch solves this problem by changing code not to 
      check if routine exists before checking if user has enough 
      privileges to drop it. Moreover we no longer perform this 
      check using a separate call instead we rely on 
      sp_drop_routine() returning SP_KEY_NOT_FOUND if routine 
      doesn't exist.
      
      This change also simplifies one of upcoming patches
      refactoring global read lock implementation.
     @ mysql-test/r/grant.result
        Updated test case after fixing bug#57061 "User without
        privilege on routine can discover its existence". Removed
        DROP PROCEDURE/FUNCTION statements which have started to
        fail after this fix (correctly). There is no need in
        dropping routines in freshly created database anyway.
     @ mysql-test/r/sp-security.result
        Added new test case for bug#57061 "User without privilege
        on routine can discover its existence". Updated existing
        tests according to new behaviour.
     @ mysql-test/suite/funcs_1/r/innodb_storedproc_06.result
        Updated test case after fixing bug#57061 "User without
        privilege on routine can discover its existence".
        Now we drop routines under user which has enough
        privileges to do so.
     @ mysql-test/suite/funcs_1/r/memory_storedproc_06.result
        Updated test case after fixing bug#57061 "User without
        privilege on routine can discover its existence".
        Now we drop routines under user which has enough
        privileges to do so.
     @ mysql-test/suite/funcs_1/r/myisam_storedproc_06.result
        Updated test case after fixing bug#57061 "User without
        privilege on routine can discover its existence".
        Now we drop routines under user which has enough
        privileges to do so.
     @ mysql-test/suite/funcs_1/storedproc/storedproc_06.inc
        Updated test case after fixing bug#57061 "User without
        privilege on routine can discover its existence".
        Now we drop routines under user which has enough
        privileges to do so.
     @ mysql-test/t/grant.test
        Updated test case after fixing bug#57061 "User without
        privilege on routine can discover its existence". Removed
        DROP PROCEDURE/FUNCTION statements which have started to
        fail after this fix (correctly). There is no need in
        dropping routines in freshly created database anyway.
     @ mysql-test/t/sp-security.test
        Added new test case for bug#57061 "User without privilege
        on routine can discover its existence". Updated existing
        tests according to new behaviour.
     @ sql/sp.cc
        Removed sp_routine_exists_in_table() which is no longer
        used.
     @ sql/sp.h
        Removed sp_routine_exists_in_table() which is no longer
        used.
     @ sql/sql_parse.cc
        When dropping routine we no longer check if routine exists 
        before checking if user has enough privileges to do so. 
        Moreover we no longer perform this check using a separate 
        call instead we rely on sp_drop_routine() returning 
        SP_KEY_NOT_FOUND if routine doesn't exist.

    modified:
      mysql-test/r/grant.result
      mysql-test/r/sp-security.result
      mysql-test/suite/funcs_1/r/innodb_storedproc_06.result
      mysql-test/suite/funcs_1/r/memory_storedproc_06.result
      mysql-test/suite/funcs_1/r/myisam_storedproc_06.result
      mysql-test/suite/funcs_1/storedproc/storedproc_06.inc
      mysql-test/t/grant.test
      mysql-test/t/sp-security.test
      sql/sp.cc
      sql/sp.h
      sql/sql_parse.cc
=== modified file 'mysql-test/r/grant.result'
--- a/mysql-test/r/grant.result	2010-08-16 15:16:07 +0000
+++ b/mysql-test/r/grant.result	2010-10-07 16:01:17 +0000
@@ -1448,8 +1448,6 @@ CREATE USER 'userbug33464'@'localhost';
 GRANT CREATE ROUTINE ON dbbug33464.* TO 'userbug33464'@'localhost';
 	
 userbug33464@localhost	dbbug33464
-DROP PROCEDURE IF EXISTS sp3;
-DROP FUNCTION IF EXISTS fn1;
 CREATE PROCEDURE sp3(v1 char(20))
 BEGIN
 SELECT * from dbbug33464.t6 where t6.f2= 'xyz';

=== modified file 'mysql-test/r/sp-security.result'
--- a/mysql-test/r/sp-security.result	2010-03-01 09:45:36 +0000
+++ b/mysql-test/r/sp-security.result	2010-10-07 16:01:17 +0000
@@ -44,7 +44,7 @@ ERROR 42000: SELECT command denied to us
 create procedure db1_secret.dummy() begin end;
 ERROR 42000: Access denied for user 'user1'@'localhost' to database 'db1_secret'
 drop procedure db1_secret.dummy;
-ERROR 42000: PROCEDURE db1_secret.dummy does not exist
+ERROR 42000: alter routine command denied to user 'user1'@'localhost' for routine 'db1_secret.dummy'
 drop procedure db1_secret.stamp;
 ERROR 42000: alter routine command denied to user 'user1'@'localhost' for routine 'db1_secret.stamp'
 drop function db1_secret.db;
@@ -58,7 +58,7 @@ ERROR 42000: SELECT command denied to us
 create procedure db1_secret.dummy() begin end;
 ERROR 42000: Access denied for user ''@'%' to database 'db1_secret'
 drop procedure db1_secret.dummy;
-ERROR 42000: PROCEDURE db1_secret.dummy does not exist
+ERROR 42000: alter routine command denied to user ''@'%' for routine 'db1_secret.dummy'
 drop procedure db1_secret.stamp;
 ERROR 42000: alter routine command denied to user ''@'%' for routine 'db1_secret.stamp'
 drop function db1_secret.db;
@@ -567,3 +567,28 @@ DROP USER 'tester';
 DROP USER 'Tester';
 DROP DATABASE B48872;
 End of 5.0 tests.
+#
+# Test for bug#57061 "User without privilege on routine can discover
+# its existence."
+#
+drop database if exists mysqltest_db;
+create database mysqltest_db;
+# Create user with no privileges on mysqltest_db database.
+create user bug57061_user@localhost;
+create function mysqltest_db.f1() returns int return 0;
+create procedure mysqltest_db.p1() begin end;
+# Connect as user 'bug57061_user@localhost'
+# Attempt to drop routine on which user doesn't have privileges
+# should result in the same 'access denied' type of error whether
+# routine exists or not.
+drop function if exists mysqltest_db.f_does_not_exist;
+ERROR 42000: alter routine command denied to user 'bug57061_user'@'localhost' for routine 'mysqltest_db.f_does_not_exist'
+drop procedure if exists mysqltest_db.p_does_not_exist;
+ERROR 42000: alter routine command denied to user 'bug57061_user'@'localhost' for routine 'mysqltest_db.p_does_not_exist'
+drop function if exists mysqltest_db.f1;
+ERROR 42000: alter routine command denied to user 'bug57061_user'@'localhost' for routine 'mysqltest_db.f1'
+drop procedure if exists mysqltest_db.p1;
+ERROR 42000: alter routine command denied to user 'bug57061_user'@'localhost' for routine 'mysqltest_db.p1'
+# Connection 'default'.
+drop user bug57061_user@localhost;
+drop database mysqltest_db;

=== modified file 'mysql-test/suite/funcs_1/r/innodb_storedproc_06.result'
--- a/mysql-test/suite/funcs_1/r/innodb_storedproc_06.result	2010-03-19 08:56:26 +0000
+++ b/mysql-test/suite/funcs_1/r/innodb_storedproc_06.result	2010-10-07 16:01:17 +0000
@@ -110,10 +110,10 @@ Ensure that root always has the GRANT CR
 --------------------------------------------------------------------------------
 grant create routine on db_storedproc_1.* to 'user_1'@'localhost';
 flush privileges;
+DROP PROCEDURE IF EXISTS db_storedproc_1.sp3;
+DROP FUNCTION IF EXISTS db_storedproc_1.fn1;
 	
 user_1@localhost	db_storedproc_1
-DROP PROCEDURE IF EXISTS sp3;
-DROP FUNCTION IF EXISTS fn1;
 CREATE PROCEDURE sp3(v1 char(20))
 BEGIN
 SELECT * from db_storedproc_1.t6 where t6.f2= 'xyz';

=== modified file 'mysql-test/suite/funcs_1/r/memory_storedproc_06.result'
--- a/mysql-test/suite/funcs_1/r/memory_storedproc_06.result	2010-03-19 08:56:26 +0000
+++ b/mysql-test/suite/funcs_1/r/memory_storedproc_06.result	2010-10-07 16:01:17 +0000
@@ -111,10 +111,10 @@ Ensure that root always has the GRANT CR
 --------------------------------------------------------------------------------
 grant create routine on db_storedproc_1.* to 'user_1'@'localhost';
 flush privileges;
+DROP PROCEDURE IF EXISTS db_storedproc_1.sp3;
+DROP FUNCTION IF EXISTS db_storedproc_1.fn1;
 	
 user_1@localhost	db_storedproc_1
-DROP PROCEDURE IF EXISTS sp3;
-DROP FUNCTION IF EXISTS fn1;
 CREATE PROCEDURE sp3(v1 char(20))
 BEGIN
 SELECT * from db_storedproc_1.t6 where t6.f2= 'xyz';

=== modified file 'mysql-test/suite/funcs_1/r/myisam_storedproc_06.result'
--- a/mysql-test/suite/funcs_1/r/myisam_storedproc_06.result	2010-03-19 08:56:26 +0000
+++ b/mysql-test/suite/funcs_1/r/myisam_storedproc_06.result	2010-10-07 16:01:17 +0000
@@ -111,10 +111,10 @@ Ensure that root always has the GRANT CR
 --------------------------------------------------------------------------------
 grant create routine on db_storedproc_1.* to 'user_1'@'localhost';
 flush privileges;
+DROP PROCEDURE IF EXISTS db_storedproc_1.sp3;
+DROP FUNCTION IF EXISTS db_storedproc_1.fn1;
 	
 user_1@localhost	db_storedproc_1
-DROP PROCEDURE IF EXISTS sp3;
-DROP FUNCTION IF EXISTS fn1;
 CREATE PROCEDURE sp3(v1 char(20))
 BEGIN
 SELECT * from db_storedproc_1.t6 where t6.f2= 'xyz';

=== modified file 'mysql-test/suite/funcs_1/storedproc/storedproc_06.inc'
--- a/mysql-test/suite/funcs_1/storedproc/storedproc_06.inc	2010-03-19 08:56:26 +0000
+++ b/mysql-test/suite/funcs_1/storedproc/storedproc_06.inc	2010-10-07 16:01:17 +0000
@@ -117,15 +117,15 @@ create user 'user_1'@'localhost';
 grant create routine on db_storedproc_1.* to 'user_1'@'localhost';
 flush privileges;
 
+--disable_warnings
+DROP PROCEDURE IF EXISTS db_storedproc_1.sp3;
+DROP FUNCTION IF EXISTS db_storedproc_1.fn1;
+--enable_warnings
+
 #  disconnect default;
 connect (user2, localhost, user_1, , db_storedproc_1);
 --source suite/funcs_1/include/show_connection.inc
 
---disable_warnings
-DROP PROCEDURE IF EXISTS sp3;
-DROP FUNCTION IF EXISTS fn1;
---enable_warnings
-
 delimiter //;
 CREATE PROCEDURE sp3(v1 char(20))
 BEGIN

=== modified file 'mysql-test/t/grant.test'
--- a/mysql-test/t/grant.test	2010-08-16 15:16:07 +0000
+++ b/mysql-test/t/grant.test	2010-10-07 16:01:17 +0000
@@ -1419,11 +1419,6 @@ GRANT CREATE ROUTINE ON dbbug33464.* TO 
 connect (connbug33464, localhost, userbug33464, , dbbug33464);
 --source suite/funcs_1/include/show_connection.inc
 
---disable_warnings
-DROP PROCEDURE IF EXISTS sp3;
-DROP FUNCTION IF EXISTS fn1;
---enable_warnings
-
 delimiter //;
 CREATE PROCEDURE sp3(v1 char(20))
 BEGIN

=== modified file 'mysql-test/t/sp-security.test'
--- a/mysql-test/t/sp-security.test	2010-02-26 13:16:46 +0000
+++ b/mysql-test/t/sp-security.test	2010-10-07 16:01:17 +0000
@@ -82,7 +82,7 @@ select * from db1_secret.t1;
 # ...and not this
 --error ER_DBACCESS_DENIED_ERROR
 create procedure db1_secret.dummy() begin end;
---error ER_SP_DOES_NOT_EXIST
+--error ER_PROCACCESS_DENIED_ERROR
 drop procedure db1_secret.dummy;
 --error ER_PROCACCESS_DENIED_ERROR
 drop procedure db1_secret.stamp;
@@ -106,7 +106,7 @@ select * from db1_secret.t1;
 # ...and not this
 --error ER_DBACCESS_DENIED_ERROR
 create procedure db1_secret.dummy() begin end;
---error ER_SP_DOES_NOT_EXIST
+--error ER_PROCACCESS_DENIED_ERROR
 drop procedure db1_secret.dummy;
 --error ER_PROCACCESS_DENIED_ERROR
 drop procedure db1_secret.stamp;
@@ -926,6 +926,39 @@ DROP DATABASE B48872;
 
 --echo End of 5.0 tests.
 
+
+--echo #
+--echo # Test for bug#57061 "User without privilege on routine can discover
+--echo # its existence."
+--echo #
+--disable_warnings
+drop database if exists mysqltest_db;
+--enable_warnings
+create database mysqltest_db;
+--echo # Create user with no privileges on mysqltest_db database.
+create user bug57061_user@localhost;
+create function mysqltest_db.f1() returns int return 0;
+create procedure mysqltest_db.p1() begin end;
+--echo # Connect as user 'bug57061_user@localhost'
+connect (conn1, localhost, bug57061_user,,);
+--echo # Attempt to drop routine on which user doesn't have privileges
+--echo # should result in the same 'access denied' type of error whether
+--echo # routine exists or not.
+--error ER_PROCACCESS_DENIED_ERROR
+drop function if exists mysqltest_db.f_does_not_exist;
+--error ER_PROCACCESS_DENIED_ERROR
+drop procedure if exists mysqltest_db.p_does_not_exist;
+--error ER_PROCACCESS_DENIED_ERROR
+drop function if exists mysqltest_db.f1;
+--error ER_PROCACCESS_DENIED_ERROR
+drop procedure if exists mysqltest_db.p1;
+--echo # Connection 'default'.
+connection default;
+disconnect conn1;
+drop user bug57061_user@localhost;
+drop database mysqltest_db;
+
+
 # Wait till all disconnects are completed
 --source include/wait_until_count_sessions.inc
 

=== modified file 'sql/sp.cc'
--- a/sql/sp.cc	2010-09-30 10:43:43 +0000
+++ b/sql/sp.cc	2010-10-07 16:01:17 +0000
@@ -1636,38 +1636,6 @@ sp_exist_routines(THD *thd, TABLE_LIST *
 }
 
 
-/**
-  Check if a routine exists in the mysql.proc table, without actually
-  parsing the definition. (Used for dropping).
-
-  @param thd          thread context
-  @param name         name of procedure
-
-  @retval
-    0       Success
-  @retval
-    non-0   Error;  SP_OPEN_TABLE_FAILED or SP_KEY_NOT_FOUND
-*/
-
-int
-sp_routine_exists_in_table(THD *thd, int type, sp_name *name)
-{
-  TABLE *table;
-  int ret;
-  Open_tables_backup open_tables_state_backup;
-
-  if (!(table= open_proc_table_for_read(thd, &open_tables_state_backup)))
-    ret= SP_OPEN_TABLE_FAILED;
-  else
-  {
-    if ((ret= db_find_routine_aux(thd, type, name, table)) != SP_OK)
-      ret= SP_KEY_NOT_FOUND;
-    close_system_tables(thd, &open_tables_state_backup);
-  }
-  return ret;
-}
-
-
 extern "C" uchar* sp_sroutine_key(const uchar *ptr, size_t *plen,
                                   my_bool first)
 {

=== modified file 'sql/sp.h'
--- a/sql/sp.h	2010-06-11 01:30:49 +0000
+++ b/sql/sp.h	2010-10-07 16:01:17 +0000
@@ -100,9 +100,6 @@ sp_cache_routine(THD *thd, int type, sp_
 bool
 sp_exist_routines(THD *thd, TABLE_LIST *procs, bool any);
 
-int
-sp_routine_exists_in_table(THD *thd, int type, sp_name *name);
-
 bool
 sp_show_create_routine(THD *thd, int type, sp_name *name);
 

=== modified file 'sql/sql_parse.cc'
--- a/sql/sql_parse.cc	2010-10-06 14:34:28 +0000
+++ b/sql/sql_parse.cc	2010-10-07 16:01:17 +0000
@@ -4023,49 +4023,39 @@ create_sp_error:
       int sp_result;
       int type= (lex->sql_command == SQLCOM_DROP_PROCEDURE ?
                  TYPE_ENUM_PROCEDURE : TYPE_ENUM_FUNCTION);
+      char *db= lex->spname->m_db.str;
+      char *name= lex->spname->m_name.str;
 
-      /*
-        @todo: here we break the metadata locking protocol by
-        looking up the information about the routine without
-        a metadata lock. Rewrite this piece to make sp_drop_routine
-        return whether the routine existed or not.
-      */
-      sp_result= sp_routine_exists_in_table(thd, type, lex->spname);
-      thd->warning_info->opt_clear_warning_info(thd->query_id);
-      if (sp_result == SP_OK)
-      {
-        char *db= lex->spname->m_db.str;
-	char *name= lex->spname->m_name.str;
-
-	if (check_routine_access(thd, ALTER_PROC_ACL, db, name,
-                                 lex->sql_command == SQLCOM_DROP_PROCEDURE, 0))
-          goto error;
+      if (check_routine_access(thd, ALTER_PROC_ACL, db, name,
+                               lex->sql_command == SQLCOM_DROP_PROCEDURE, 0))
+        goto error;
 
-        /* Conditionally writes to binlog */
-        sp_result= sp_drop_routine(thd, type, lex->spname);
+      /* Conditionally writes to binlog */
+      sp_result= sp_drop_routine(thd, type, lex->spname);
 
 #ifndef NO_EMBEDDED_ACCESS_CHECKS
-        /*
-          We're going to issue an implicit REVOKE statement.
-          It takes metadata locks and updates system tables.
-          Make sure that sp_create_routine() did not leave any
-          locks in the MDL context, so there is no risk to
-          deadlock.
-        */
-        close_mysql_tables(thd);
+      /*
+        We're going to issue an implicit REVOKE statement.
+        It takes metadata locks and updates system tables.
+        Make sure that sp_create_routine() did not leave any
+        locks in the MDL context, so there is no risk to
+        deadlock.
+      */
+      close_mysql_tables(thd);
 
-        if (sp_automatic_privileges && !opt_noacl &&
-            sp_revoke_privileges(thd, db, name,
-                                 lex->sql_command == SQLCOM_DROP_PROCEDURE))
-        {
-          push_warning(thd, MYSQL_ERROR::WARN_LEVEL_WARN,
-                       ER_PROC_AUTO_REVOKE_FAIL,
-                       ER(ER_PROC_AUTO_REVOKE_FAIL));
-          /* If this happens, an error should have been reported. */
-          goto error;
-        }
-#endif
+      if (sp_result != SP_KEY_NOT_FOUND &&
+          sp_automatic_privileges && !opt_noacl &&
+          sp_revoke_privileges(thd, db, name,
+                               lex->sql_command == SQLCOM_DROP_PROCEDURE))
+      {
+        push_warning(thd, MYSQL_ERROR::WARN_LEVEL_WARN,
+                     ER_PROC_AUTO_REVOKE_FAIL,
+                     ER(ER_PROC_AUTO_REVOKE_FAIL));
+        /* If this happens, an error should have been reported. */
+        goto error;
       }
+#endif
+
       res= sp_result;
       switch (sp_result) {
       case SP_OK:


Attachment: [text/bzr-bundle] bzr/dmitry.lenev@oracle.com-20101007160117-3tmmh2kayc229act.bundle
Thread
bzr commit into mysql-5.5-runtime branch (Dmitry.Lenev:3159) Bug#57061Dmitry Lenev7 Oct