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. This is needed as a error handler already
is used in the call stack emitting the errors which needs to be converted.


mysql-test/r/grant.result:
  * Added test case for bug44658
mysql-test/t/grant.test:
  * Added test case for bug44658
sql/sp.cc:
  * Removed non functional parameter no_error and my_error calls as all errors
    from this function will be converted to a warning anyway.
  * Change function return type from int to bool.
sql/sp.h:
  * Removed non functional parameter no_error and my_error calls as all errors
    from this function will be converted to a warning anyway.
  * Changed function return value from int to bool
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 the parameter write_to_binlog
  * 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.
  * Changed the broken error state in sp_grant_privileges() to a warning
    so that if "CREATE PROCEDURE" fails because "Password hash isn't a hexidecimal
    number" it is still clear what happened.
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.
  * Introduced the parameter write_to_binlog
  * Changed return type for sp_grant_privileges() from int to bool
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.

1 files changed, 42 insertions, 40 deletions

diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc
index b1dbb7031ce..83cf37e32dc 100644
--- a/sql/sql_acl.cc
+++ b/sql/sql_acl.cc
@@ -3191,26 +3191,24 @@ int mysql_table_grant(THD *thd, TABLE_LIST *table_list,
 }
 
 
-/*
+/**
   Store routine level grants in the privilege tables
 
-  SYNOPSIS
-    mysql_routine_grant()
-    thd			Thread handle
-    table_list		List of routines to give grant
-    is_proc             true indicates routine list are procedures
-    user_list		List of users to give grant
-    rights		Table level grant
-    revoke_grant	Set to 1 if this is a REVOKE command
+  @param thd Thread handle
+  @param table_list List of routines to give grant
+  @param is_proc Is this a list of procedures?
+  @param user_list List of users to give grant
+  @param rights Table level grant
+  @param revoke_grant Is this is a REVOKE command?
 
-  RETURN
-    0	ok
-    1	error
+  @return
+    @retval FALSE Success.
+    @retval TRUE An error occurred.
 */
 
 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, bool write_to_binlog)
 {
   List_iterator <LEX_USER> str_list (user_list);
   LEX_USER *Str, *tmp_Str;
@@ -3221,22 +3219,20 @@ bool mysql_routine_grant(THD *thd, TABLE_LIST *table_list, bool is_proc,
 
   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))
       DBUG_RETURN(TRUE);
   }
 
@@ -3317,9 +3313,8 @@ bool mysql_routine_grant(THD *thd, TABLE_LIST *table_list, bool is_proc,
     {
       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 +3339,14 @@ bool mysql_routine_grant(THD *thd, TABLE_LIST *table_list, bool is_proc,
   }
   thd->mem_root= old_root;
   pthread_mutex_unlock(&acl_cache->lock);
-  if (!result && !no_error)
+
+  if (write_to_binlog)
   {
     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);
 }
@@ -6150,21 +6143,20 @@ bool sp_revoke_privileges(THD *thd, const char *sp_db, const char *sp_name,
 }
 
 
-/*
+/**
   Grant EXECUTE,ALTER privilege for a stored procedure
 
-  SYNOPSIS
-    sp_grant_privileges()
-    thd                         The current thread.
-    db				DB of the stored procedure
-    name			Name of the stored procedure
+  @param thd The current thread.
+  @param sp_db
+  @param sp_name
+  @param is_proc
 
-  RETURN
-    0           OK.
-    < 0         Error. Error message not yet sent.
+  @return
+    @retval FALSE Success
+    @retval TRUE An error occured. Error message not yet sent.
 */
 
-int sp_grant_privileges(THD *thd, const char *sp_db, const char *sp_name,
+bool sp_grant_privileges(THD *thd, const char *sp_db, const char *sp_name,
                          bool is_proc)
 {
   Security_context *sctx= thd->security_ctx;
@@ -6174,6 +6166,7 @@ int sp_grant_privileges(THD *thd, const char *sp_db, const char *sp_name,
   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,8 +6217,11 @@ int sp_grant_privileges(THD *thd, const char *sp_db, const char *sp_name,
     }
     else
     {
-      my_error(ER_PASSWD_LENGTH, MYF(0), SCRAMBLED_PASSWORD_CHAR_LENGTH);
-      return -1;
+      push_warning_printf(thd, MYSQL_ERROR::WARN_LEVEL_WARN,
+                          ER_PASSWD_LENGTH,
+                          ER(ER_PASSWD_LENGTH),
+                          SCRAMBLED_PASSWORD_CHAR_LENGTH);
+      return TRUE;
     }
     combo->password.str= passwd_buff;
   }
@@ -6241,8 +6237,14 @@ int sp_grant_privileges(THD *thd, const char *sp_db, const char *sp_name,
   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, FALSE, FALSE);
+  thd->pop_internal_handler();
   DBUG_RETURN(result);
 }