MySQL Lists are EOL. Please join:

List:Commits« Previous MessageNext Message »
From:Kristofer Pettersson Date:May 28 2009 10:36am
Subject:bzr commit into mysql-5.1-bugteam branch (kristofer.pettersson:2874)
Bug#44658
View as plain text  
#At file:///Users/thek/Development/51-bug44658/ based on revid:mats@stripped

 2874 Kristofer Pettersson	2009-05-28
      Bug#44658 Create procedure makes server crash when user does not have ALL privilege
      
      MySQL crashes if a user without proper privileges attempts to create a procedure.
      
      The crash happens because more than one error state is pushed onto the Diagnostic
      area. In this particular case the user is denied to implicitly create a new user
      account with the implicitly granted privileges ALTER- and EXECUTE ROUTINE.
      
      The new account is needed if the original user account contained a host mask.
      A user account with a host mask is a distinct user account in this context.
      An alternative would be to first get the most permissive user account which
      include the current user connection and then assign privileges to that
      account. This behavior change is considered out of scope for this bug patch.
      
      The implicit assignment of privileges when a user creates a stored routine is a
      considered to be a feature for user convenience and as such it is not
      a critical operation. Any failure to complete this operation is thus considered
      non-fatal (an error becomes a warning).
       
      The patch back ports a stack implementation of the internal error handler interface.
      This enables the use of multiple error handlers so that it is possible to intercept and
      cancel errors thrown by lower layers. 
     @ mysql-test/r/grant.result
        * Added test case for bug44658
     @ mysql-test/t/grant.test
        * Added test case for bug44658
     @ sql/sql_acl.cc
        * Removed the non functional no_error parameter from the function prototype.
        The function is called from two places and in one of the places we now 
        ignore errors through error handlers.
        * Introduced an error handler to cancel any error state from mysql_routine_grant.
        * Moved my_ok() signal from mysql_routine_grant to make it easier to avoid
        setting the wrong state in the Diagnostic area.
     @ sql/sql_acl.h
        * Removed the non functional no_error parameter from the function prototype.
        The function is called from two places and in one of the places we now 
        ignore errors through error handlers.
     @ sql/sql_class.cc
        * Back ported implementation of internal error handler from 6.0 branch
     @ sql/sql_class.h
        * Back ported implementation of internal error handler from 6.0 branch
     @ sql/sql_parse.cc
        * Moved my_ok() signal from mysql_routine grant to make it easier to avoid
        setting the wrong state in the Diagnostic area.

    modified:
      mysql-test/r/grant.result
      mysql-test/t/grant.test
      sql/sql_acl.cc
      sql/sql_acl.h
      sql/sql_class.cc
      sql/sql_class.h
      sql/sql_parse.cc
=== modified file 'mysql-test/r/grant.result'
--- a/mysql-test/r/grant.result	2009-02-25 12:18:24 +0000
+++ b/mysql-test/r/grant.result	2009-05-28 10:36:23 +0000
@@ -1358,3 +1358,58 @@ DROP USER 'userbug33464'@'localhost';
 USE test;
 DROP DATABASE dbbug33464;
 SET @@global.log_bin_trust_function_creators= @old_log_bin_trust_function_creators;
+CREATE USER user1;
+CREATE USER user2;
+GRANT CREATE ON db1.* TO 'user1'@'localhost';
+GRANT CREATE ROUTINE ON db1.* TO 'user1'@'localhost';
+GRANT CREATE ON db1.* TO 'user2'@'%';
+GRANT CREATE ROUTINE ON db1.* TO 'user2'@'%';
+FLUSH PRIVILEGES;
+SHOW GRANTS FOR 'user1'@'localhost';
+Grants for user1@localhost
+GRANT USAGE ON *.* TO 'user1'@'localhost'
+GRANT CREATE, CREATE ROUTINE ON `db1`.* TO 'user1'@'localhost'
+** Connect as user1 and create a procedure.
+** The creation will imply implicitly assigned
+** EXECUTE and ALTER ROUTINE privileges to
+** the current user user1@localhost. 
+SELECT @@GLOBAL.sql_mode;
+@@GLOBAL.sql_mode
+
+SELECT @@SESSION.sql_mode;
+@@SESSION.sql_mode
+
+CREATE DATABASE db1;
+CREATE PROCEDURE db1.proc1(p1 INT)
+BEGIN
+SET @x = 0;
+REPEAT SET @x = @x + 1; UNTIL @x > p1 END REPEAT;
+END ;||
+** Connect as user2 and create a procedure.
+** Implicitly assignment of privileges will
+** fail because the user2@localhost is an
+** unknown user.
+CREATE PROCEDURE db1.proc2(p1 INT)
+BEGIN
+SET @x = 0;
+REPEAT SET @x = @x + 1; UNTIL @x > p1 END REPEAT;
+END ;||
+Warnings:
+Warning	1404	Failed to grant EXECUTE and ALTER ROUTINE privileges
+SHOW GRANTS FOR 'user1'@'localhost';
+Grants for user1@localhost
+GRANT USAGE ON *.* TO 'user1'@'localhost'
+GRANT CREATE, CREATE ROUTINE ON `db1`.* TO 'user1'@'localhost'
+GRANT EXECUTE, ALTER ROUTINE ON PROCEDURE `db1`.`proc1` TO 'user1'@'localhost'
+SHOW GRANTS FOR 'user2';
+Grants for user2@%
+GRANT USAGE ON *.* TO 'user2'@'%'
+GRANT CREATE, CREATE ROUTINE ON `db1`.* TO 'user2'@'%'
+DROP PROCEDURE db1.proc1;
+DROP PROCEDURE db1.proc2;
+REVOKE ALL ON db1.* FROM 'user1'@'localhost';
+REVOKE ALL ON db1.* FROM 'user2'@'%';
+DROP USER 'user1';
+DROP USER 'user1'@'localhost';
+DROP USER 'user2';
+DROP DATABASE db1;

=== modified file 'mysql-test/t/grant.test'
--- a/mysql-test/t/grant.test	2009-02-10 10:23:47 +0000
+++ b/mysql-test/t/grant.test	2009-05-28 10:36:23 +0000
@@ -1471,5 +1471,59 @@ DROP DATABASE dbbug33464;
 
 SET @@global.log_bin_trust_function_creators= @old_log_bin_trust_function_creators;
 
+#
+# Bug#44658 Create procedure makes server crash when user does not have ALL privilege
+#
+CREATE USER user1;
+CREATE USER user2;
+GRANT CREATE ON db1.* TO 'user1'@'localhost';
+GRANT CREATE ROUTINE ON db1.* TO 'user1'@'localhost';
+GRANT CREATE ON db1.* TO 'user2'@'%';
+GRANT CREATE ROUTINE ON db1.* TO 'user2'@'%';
+FLUSH PRIVILEGES;
+SHOW GRANTS FOR 'user1'@'localhost';
+connect (con1,localhost,user1,,);
+--echo ** Connect as user1 and create a procedure.
+--echo ** The creation will imply implicitly assigned
+--echo ** EXECUTE and ALTER ROUTINE privileges to
+--echo ** the current user user1@localhost. 
+SELECT @@GLOBAL.sql_mode;
+SELECT @@SESSION.sql_mode;
+CREATE DATABASE db1;
+DELIMITER ||;
+CREATE PROCEDURE db1.proc1(p1 INT)
+ BEGIN
+ SET @x = 0;
+ REPEAT SET @x = @x + 1; UNTIL @x > p1 END REPEAT;
+ END ;||
+DELIMITER ;||
+
+connect (con2,localhost,user2,,);
+--echo ** Connect as user2 and create a procedure.
+--echo ** Implicitly assignment of privileges will
+--echo ** fail because the user2@localhost is an
+--echo ** unknown user.
+DELIMITER ||;
+CREATE PROCEDURE db1.proc2(p1 INT)
+ BEGIN
+ SET @x = 0;
+ REPEAT SET @x = @x + 1; UNTIL @x > p1 END REPEAT;
+ END ;||
+DELIMITER ;||
+
+connection default;
+SHOW GRANTS FOR 'user1'@'localhost';
+SHOW GRANTS FOR 'user2';
+disconnect con1;
+disconnect con2;
+DROP PROCEDURE db1.proc1;
+DROP PROCEDURE db1.proc2;
+REVOKE ALL ON db1.* FROM 'user1'@'localhost';
+REVOKE ALL ON db1.* FROM 'user2'@'%';
+DROP USER 'user1';
+DROP USER 'user1'@'localhost';
+DROP USER 'user2';
+DROP DATABASE db1;
+
 # Wait till we reached the initial number of concurrent sessions
 --source include/wait_until_count_sessions.inc

=== modified file 'sql/sql_acl.cc'
--- a/sql/sql_acl.cc	2009-04-08 23:42:51 +0000
+++ b/sql/sql_acl.cc	2009-05-28 10:36:23 +0000
@@ -3210,7 +3210,7 @@ int mysql_table_grant(THD *thd, TABLE_LI
 
 bool mysql_routine_grant(THD *thd, TABLE_LIST *table_list, bool is_proc,
 			 List <LEX_USER> &user_list, ulong rights,
-			 bool revoke_grant, bool no_error)
+			 bool revoke_grant)
 {
   List_iterator <LEX_USER> str_list (user_list);
   LEX_USER *Str, *tmp_Str;
@@ -3221,22 +3221,20 @@ bool mysql_routine_grant(THD *thd, TABLE
 
   if (!initialized)
   {
-    if (!no_error)
-      my_error(ER_OPTION_PREVENTS_STATEMENT, MYF(0),
-               "--skip-grant-tables");
+    my_error(ER_OPTION_PREVENTS_STATEMENT, MYF(0),
+             "--skip-grant-tables");
     DBUG_RETURN(TRUE);
   }
   if (rights & ~PROC_ACLS)
   {
-    if (!no_error)
-      my_message(ER_ILLEGAL_GRANT_FOR_TABLE, ER(ER_ILLEGAL_GRANT_FOR_TABLE),
-        	 MYF(0));
+    my_message(ER_ILLEGAL_GRANT_FOR_TABLE, ER(ER_ILLEGAL_GRANT_FOR_TABLE),
+               MYF(0));
     DBUG_RETURN(TRUE);
   }
 
   if (!revoke_grant)
   {
-    if (sp_exist_routines(thd, table_list, is_proc, no_error)<0)
+    if (sp_exist_routines(thd, table_list, is_proc, FALSE)<0)
       DBUG_RETURN(TRUE);
   }
 
@@ -3317,9 +3315,8 @@ bool mysql_routine_grant(THD *thd, TABLE
     {
       if (revoke_grant)
       {
-        if (!no_error)
-          my_error(ER_NONEXISTING_PROC_GRANT, MYF(0),
-		   Str->user.str, Str->host.str, table_name);
+        my_error(ER_NONEXISTING_PROC_GRANT, MYF(0),
+	         Str->user.str, Str->host.str, table_name);
 	result= TRUE;
 	continue;
       }
@@ -3344,16 +3341,13 @@ bool mysql_routine_grant(THD *thd, TABLE
   }
   thd->mem_root= old_root;
   pthread_mutex_unlock(&acl_cache->lock);
-  if (!result && !no_error)
+  if (!result)
   {
     write_bin_log(thd, TRUE, thd->query, thd->query_length);
   }
 
   rw_unlock(&LOCK_grant);
 
-  if (!result && !no_error)
-    my_ok(thd);
-
   /* Tables are automatically closed */
   DBUG_RETURN(result);
 }
@@ -6174,6 +6168,7 @@ int sp_grant_privileges(THD *thd, const 
   bool result;
   ACL_USER *au;
   char passwd_buff[SCRAMBLED_PASSWORD_CHAR_LENGTH+1];
+  Dummy_error_handler error_handler;
   DBUG_ENTER("sp_grant_privileges");
 
   if (!(combo=(LEX_USER*) thd->alloc(sizeof(st_lex_user))))
@@ -6224,7 +6219,6 @@ int sp_grant_privileges(THD *thd, const 
     }
     else
     {
-      my_error(ER_PASSWD_LENGTH, MYF(0), SCRAMBLED_PASSWORD_CHAR_LENGTH);
       return -1;
     }
     combo->password.str= passwd_buff;
@@ -6241,8 +6235,14 @@ int sp_grant_privileges(THD *thd, const 
   thd->lex->ssl_type= SSL_TYPE_NOT_SPECIFIED;
   bzero((char*) &thd->lex->mqh, sizeof(thd->lex->mqh));
 
+  /*
+    Only care about whether the operation failed or succeeded
+    as all errors will be handled later.
+  */
+  thd->push_internal_handler(&error_handler);
   result= mysql_routine_grant(thd, tables, is_proc, user_list,
-  				DEFAULT_CREATE_PROC_ACLS, 0, 1);
+                              DEFAULT_CREATE_PROC_ACLS, 0);
+  thd->pop_internal_handler();
   DBUG_RETURN(result);
 }
 

=== modified file 'sql/sql_acl.h'
--- a/sql/sql_acl.h	2008-03-21 15:48:28 +0000
+++ b/sql/sql_acl.h	2009-05-28 10:36:23 +0000
@@ -233,7 +233,7 @@ int mysql_table_grant(THD *thd, TABLE_LI
                        bool revoke);
 bool mysql_routine_grant(THD *thd, TABLE_LIST *table, bool is_proc,
 			 List <LEX_USER> &user_list, ulong rights,
-			 bool revoke, bool no_error);
+			 bool revoke);
 my_bool grant_init();
 void grant_free(void);
 my_bool grant_reload(THD *thd);

=== modified file 'sql/sql_class.cc'
--- a/sql/sql_class.cc	2009-05-10 16:20:35 +0000
+++ b/sql/sql_class.cc	2009-05-28 10:36:23 +0000
@@ -674,31 +674,40 @@ THD::THD()
 
 void THD::push_internal_handler(Internal_error_handler *handler)
 {
-  /*
-    TODO: The current implementation is limited to 1 handler at a time only.
-    THD and sp_rcontext need to be modified to use a common handler stack.
-  */
-  DBUG_ASSERT(m_internal_handler == NULL);
-  m_internal_handler= handler;
+  if(m_internal_handler)
+  {
+    handler->m_prev_internal_handler= m_internal_handler;
+    m_internal_handler= handler;
+  }
+  else
+  {
+    m_internal_handler= handler;
+  }
 }
 
 
 bool THD::handle_error(uint sql_errno, const char *message,
                        MYSQL_ERROR::enum_warning_level level)
 {
-  if (m_internal_handler)
+  if (!m_internal_handler)
+    return FALSE;
+
+  for(Internal_error_handler *error_handler= m_internal_handler;
+      error_handler;
+      error_handler= m_internal_handler->m_prev_internal_handler)
   {
-    return m_internal_handler->handle_error(sql_errno, message, level, this);
+    if (error_handler->handle_error(sql_errno, message, level, this))
+    return TRUE;
   }
 
-  return FALSE;                                 // 'FALSE', as per coding style
+  return FALSE;
 }
 
 
 void THD::pop_internal_handler()
 {
   DBUG_ASSERT(m_internal_handler != NULL);
-  m_internal_handler= NULL;
+  m_internal_handler= m_internal_handler->m_prev_internal_handler;
 }
 
 extern "C"

=== modified file 'sql/sql_class.h'
--- a/sql/sql_class.h	2009-03-25 19:41:16 +0000
+++ b/sql/sql_class.h	2009-05-28 10:36:23 +0000
@@ -1036,7 +1036,10 @@ show_system_thread(enum_thread_type thre
 class Internal_error_handler
 {
 protected:
-  Internal_error_handler() {}
+  Internal_error_handler() :
+    m_prev_internal_handler(NULL)
+  {}
+
   virtual ~Internal_error_handler() {}
 
 public:
@@ -1069,6 +1072,28 @@ public:
                             const char *message,
                             MYSQL_ERROR::enum_warning_level level,
                             THD *thd) = 0;
+private:
+  Internal_error_handler *m_prev_internal_handler;
+  friend class THD;
+};
+
+
+/**
+  Implements the trivial error handler which cancels all error states
+  and prevents an SQLSTATE to be set.
+*/
+
+class Dummy_error_handler : public Internal_error_handler
+{
+public:
+  bool handle_error(uint sql_errno,
+                    const char *message,
+                    MYSQL_ERROR::enum_warning_level level,
+                    THD *thd)
+  {
+    /* Ignore error */
+    return TRUE;
+  }
 };
 
 
@@ -2210,6 +2235,9 @@ public:
   thd_scheduler scheduler;
 
 public:
+  inline Internal_error_handler *get_internal_handler()
+  { return m_internal_handler; }
+
   /**
     Add an internal error handler to the thread execution context.
     @param handler the exception handler to add

=== modified file 'sql/sql_parse.cc'
--- a/sql/sql_parse.cc	2009-04-09 11:30:44 +0000
+++ b/sql/sql_parse.cc	2009-05-28 10:36:23 +0000
@@ -3883,7 +3883,9 @@ end_with_restore_list:
         res= mysql_routine_grant(thd, all_tables,
                                  lex->type == TYPE_ENUM_PROCEDURE, 
                                  lex->users_list, grants,
-                                 lex->sql_command == SQLCOM_REVOKE, 0);
+                                 lex->sql_command == SQLCOM_REVOKE);
+        if (!res)
+          my_ok(thd);
       }
       else
       {


Attachment: [text/bzr-bundle]
Thread
bzr commit into mysql-5.1-bugteam branch (kristofer.pettersson:2874)Bug#44658Kristofer Pettersson28 May
  • Re: bzr commit into mysql-5.1-bugteam branch(kristofer.pettersson:2874) Bug#44658Davi Arnaut28 May