Commit 66e0ee66 authored by Kristofer Pettersson's avatar Kristofer Pettersson

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.
parent 206bdd67
...@@ -1358,3 +1358,58 @@ DROP USER 'userbug33464'@'localhost'; ...@@ -1358,3 +1358,58 @@ DROP USER 'userbug33464'@'localhost';
USE test; USE test;
DROP DATABASE dbbug33464; DROP DATABASE dbbug33464;
SET @@global.log_bin_trust_function_creators= @old_log_bin_trust_function_creators; 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;
...@@ -1471,5 +1471,59 @@ DROP DATABASE dbbug33464; ...@@ -1471,5 +1471,59 @@ DROP DATABASE dbbug33464;
SET @@global.log_bin_trust_function_creators= @old_log_bin_trust_function_creators; 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 # Wait till we reached the initial number of concurrent sessions
--source include/wait_until_count_sessions.inc --source include/wait_until_count_sessions.inc
...@@ -1308,13 +1308,20 @@ sp_find_routine(THD *thd, int type, sp_name *name, sp_cache **cp, ...@@ -1308,13 +1308,20 @@ sp_find_routine(THD *thd, int type, sp_name *name, sp_cache **cp,
/** /**
This is used by sql_acl.cc:mysql_routine_grant() and is used to find This is used by sql_acl.cc:mysql_routine_grant() and is used to find
the routines in 'routines'. the routines in 'routines'.
@param thd Thread handler
@param routines List of needles in the hay stack
@param any Any of the needles are good enough
@return
@retval FALSE Found.
@retval TRUE Not found
*/ */
int bool
sp_exist_routines(THD *thd, TABLE_LIST *routines, bool any, bool no_error) sp_exist_routines(THD *thd, TABLE_LIST *routines, bool any)
{ {
TABLE_LIST *routine; TABLE_LIST *routine;
bool result= 0;
bool sp_object_found; bool sp_object_found;
DBUG_ENTER("sp_exists_routine"); DBUG_ENTER("sp_exists_routine");
for (routine= routines; routine; routine= routine->next_global) for (routine= routines; routine; routine= routine->next_global)
...@@ -1336,21 +1343,16 @@ sp_exist_routines(THD *thd, TABLE_LIST *routines, bool any, bool no_error) ...@@ -1336,21 +1343,16 @@ sp_exist_routines(THD *thd, TABLE_LIST *routines, bool any, bool no_error)
if (sp_object_found) if (sp_object_found)
{ {
if (any) if (any)
DBUG_RETURN(1); break;
result= 1;
} }
else if (!any) else if (!any)
{ {
if (!no_error) my_error(ER_SP_DOES_NOT_EXIST, MYF(0), "FUNCTION or PROCEDURE",
{ routine->table_name);
my_error(ER_SP_DOES_NOT_EXIST, MYF(0), "FUNCTION or PROCEDURE", DBUG_RETURN(TRUE);
routine->table_name);
DBUG_RETURN(-1);
}
DBUG_RETURN(0);
} }
} }
DBUG_RETURN(result); DBUG_RETURN(FALSE);
} }
......
...@@ -39,8 +39,8 @@ sp_head * ...@@ -39,8 +39,8 @@ sp_head *
sp_find_routine(THD *thd, int type, sp_name *name, sp_find_routine(THD *thd, int type, sp_name *name,
sp_cache **cp, bool cache_only); sp_cache **cp, bool cache_only);
int bool
sp_exist_routines(THD *thd, TABLE_LIST *procs, bool any, bool no_error); sp_exist_routines(THD *thd, TABLE_LIST *procs, bool any);
int int
sp_routine_exists_in_table(THD *thd, int type, sp_name *name); sp_routine_exists_in_table(THD *thd, int type, sp_name *name);
......
...@@ -3191,26 +3191,24 @@ int mysql_table_grant(THD *thd, TABLE_LIST *table_list, ...@@ -3191,26 +3191,24 @@ int mysql_table_grant(THD *thd, TABLE_LIST *table_list,
} }
/* /**
Store routine level grants in the privilege tables Store routine level grants in the privilege tables
SYNOPSIS @param thd Thread handle
mysql_routine_grant() @param table_list List of routines to give grant
thd Thread handle @param is_proc Is this a list of procedures?
table_list List of routines to give grant @param user_list List of users to give grant
is_proc true indicates routine list are procedures @param rights Table level grant
user_list List of users to give grant @param revoke_grant Is this is a REVOKE command?
rights Table level grant
revoke_grant Set to 1 if this is a REVOKE command
RETURN @return
0 ok @retval FALSE Success.
1 error @retval TRUE An error occurred.
*/ */
bool mysql_routine_grant(THD *thd, TABLE_LIST *table_list, bool is_proc, bool mysql_routine_grant(THD *thd, TABLE_LIST *table_list, bool is_proc,
List <LEX_USER> &user_list, ulong rights, 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); List_iterator <LEX_USER> str_list (user_list);
LEX_USER *Str, *tmp_Str; LEX_USER *Str, *tmp_Str;
...@@ -3221,22 +3219,20 @@ bool mysql_routine_grant(THD *thd, TABLE_LIST *table_list, bool is_proc, ...@@ -3221,22 +3219,20 @@ bool mysql_routine_grant(THD *thd, TABLE_LIST *table_list, bool is_proc,
if (!initialized) if (!initialized)
{ {
if (!no_error) my_error(ER_OPTION_PREVENTS_STATEMENT, MYF(0),
my_error(ER_OPTION_PREVENTS_STATEMENT, MYF(0), "--skip-grant-tables");
"--skip-grant-tables");
DBUG_RETURN(TRUE); DBUG_RETURN(TRUE);
} }
if (rights & ~PROC_ACLS) if (rights & ~PROC_ACLS)
{ {
if (!no_error) my_message(ER_ILLEGAL_GRANT_FOR_TABLE, ER(ER_ILLEGAL_GRANT_FOR_TABLE),
my_message(ER_ILLEGAL_GRANT_FOR_TABLE, ER(ER_ILLEGAL_GRANT_FOR_TABLE), MYF(0));
MYF(0));
DBUG_RETURN(TRUE); DBUG_RETURN(TRUE);
} }
if (!revoke_grant) 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); DBUG_RETURN(TRUE);
} }
...@@ -3317,9 +3313,8 @@ bool mysql_routine_grant(THD *thd, TABLE_LIST *table_list, bool is_proc, ...@@ -3317,9 +3313,8 @@ bool mysql_routine_grant(THD *thd, TABLE_LIST *table_list, bool is_proc,
{ {
if (revoke_grant) if (revoke_grant)
{ {
if (!no_error) my_error(ER_NONEXISTING_PROC_GRANT, MYF(0),
my_error(ER_NONEXISTING_PROC_GRANT, MYF(0), Str->user.str, Str->host.str, table_name);
Str->user.str, Str->host.str, table_name);
result= TRUE; result= TRUE;
continue; continue;
} }
...@@ -3344,16 +3339,14 @@ bool mysql_routine_grant(THD *thd, TABLE_LIST *table_list, bool is_proc, ...@@ -3344,16 +3339,14 @@ bool mysql_routine_grant(THD *thd, TABLE_LIST *table_list, bool is_proc,
} }
thd->mem_root= old_root; thd->mem_root= old_root;
pthread_mutex_unlock(&acl_cache->lock); pthread_mutex_unlock(&acl_cache->lock);
if (!result && !no_error)
if (write_to_binlog)
{ {
write_bin_log(thd, TRUE, thd->query, thd->query_length); write_bin_log(thd, TRUE, thd->query, thd->query_length);
} }
rw_unlock(&LOCK_grant); rw_unlock(&LOCK_grant);
if (!result && !no_error)
my_ok(thd);
/* Tables are automatically closed */ /* Tables are automatically closed */
DBUG_RETURN(result); DBUG_RETURN(result);
} }
...@@ -6150,21 +6143,20 @@ bool sp_revoke_privileges(THD *thd, const char *sp_db, const char *sp_name, ...@@ -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 Grant EXECUTE,ALTER privilege for a stored procedure
SYNOPSIS @param thd The current thread.
sp_grant_privileges() @param sp_db
thd The current thread. @param sp_name
db DB of the stored procedure @param is_proc
name Name of the stored procedure
RETURN @return
0 OK. @retval FALSE Success
< 0 Error. Error message not yet sent. @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) bool is_proc)
{ {
Security_context *sctx= thd->security_ctx; Security_context *sctx= thd->security_ctx;
...@@ -6174,6 +6166,7 @@ int sp_grant_privileges(THD *thd, const char *sp_db, const char *sp_name, ...@@ -6174,6 +6166,7 @@ int sp_grant_privileges(THD *thd, const char *sp_db, const char *sp_name,
bool result; bool result;
ACL_USER *au; ACL_USER *au;
char passwd_buff[SCRAMBLED_PASSWORD_CHAR_LENGTH+1]; char passwd_buff[SCRAMBLED_PASSWORD_CHAR_LENGTH+1];
Dummy_error_handler error_handler;
DBUG_ENTER("sp_grant_privileges"); DBUG_ENTER("sp_grant_privileges");
if (!(combo=(LEX_USER*) thd->alloc(sizeof(st_lex_user)))) 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, ...@@ -6224,8 +6217,11 @@ int sp_grant_privileges(THD *thd, const char *sp_db, const char *sp_name,
} }
else else
{ {
my_error(ER_PASSWD_LENGTH, MYF(0), SCRAMBLED_PASSWORD_CHAR_LENGTH); push_warning_printf(thd, MYSQL_ERROR::WARN_LEVEL_WARN,
return -1; ER_PASSWD_LENGTH,
ER(ER_PASSWD_LENGTH),
SCRAMBLED_PASSWORD_CHAR_LENGTH);
return TRUE;
} }
combo->password.str= passwd_buff; combo->password.str= passwd_buff;
} }
...@@ -6241,8 +6237,14 @@ int sp_grant_privileges(THD *thd, const char *sp_db, const char *sp_name, ...@@ -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; thd->lex->ssl_type= SSL_TYPE_NOT_SPECIFIED;
bzero((char*) &thd->lex->mqh, sizeof(thd->lex->mqh)); 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, 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); DBUG_RETURN(result);
} }
......
...@@ -233,7 +233,7 @@ int mysql_table_grant(THD *thd, TABLE_LIST *table, List <LEX_USER> &user_list, ...@@ -233,7 +233,7 @@ int mysql_table_grant(THD *thd, TABLE_LIST *table, List <LEX_USER> &user_list,
bool revoke); bool revoke);
bool mysql_routine_grant(THD *thd, TABLE_LIST *table, bool is_proc, bool mysql_routine_grant(THD *thd, TABLE_LIST *table, bool is_proc,
List <LEX_USER> &user_list, ulong rights, List <LEX_USER> &user_list, ulong rights,
bool revoke, bool no_error); bool revoke, bool write_to_binlog);
my_bool grant_init(); my_bool grant_init();
void grant_free(void); void grant_free(void);
my_bool grant_reload(THD *thd); my_bool grant_reload(THD *thd);
...@@ -264,7 +264,7 @@ void fill_effective_table_privileges(THD *thd, GRANT_INFO *grant, ...@@ -264,7 +264,7 @@ void fill_effective_table_privileges(THD *thd, GRANT_INFO *grant,
const char *db, const char *table); const char *db, const char *table);
bool sp_revoke_privileges(THD *thd, const char *sp_db, const char *sp_name, bool sp_revoke_privileges(THD *thd, const char *sp_db, const char *sp_name,
bool is_proc); bool is_proc);
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); bool is_proc);
bool check_routine_level_acl(THD *thd, const char *db, const char *name, bool check_routine_level_acl(THD *thd, const char *db, const char *name,
bool is_proc); bool is_proc);
......
...@@ -674,31 +674,40 @@ THD::THD() ...@@ -674,31 +674,40 @@ THD::THD()
void THD::push_internal_handler(Internal_error_handler *handler) void THD::push_internal_handler(Internal_error_handler *handler)
{ {
/* if (m_internal_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. handler->m_prev_internal_handler= m_internal_handler;
*/ m_internal_handler= handler;
DBUG_ASSERT(m_internal_handler == NULL); }
m_internal_handler= handler; else
{
m_internal_handler= handler;
}
} }
bool THD::handle_error(uint sql_errno, const char *message, bool THD::handle_error(uint sql_errno, const char *message,
MYSQL_ERROR::enum_warning_level level) 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() void THD::pop_internal_handler()
{ {
DBUG_ASSERT(m_internal_handler != NULL); DBUG_ASSERT(m_internal_handler != NULL);
m_internal_handler= NULL; m_internal_handler= m_internal_handler->m_prev_internal_handler;
} }
extern "C" extern "C"
......
...@@ -1036,7 +1036,10 @@ show_system_thread(enum_thread_type thread) ...@@ -1036,7 +1036,10 @@ show_system_thread(enum_thread_type thread)
class Internal_error_handler class Internal_error_handler
{ {
protected: protected:
Internal_error_handler() {} Internal_error_handler() :
m_prev_internal_handler(NULL)
{}
virtual ~Internal_error_handler() {} virtual ~Internal_error_handler() {}
public: public:
...@@ -1069,6 +1072,28 @@ public: ...@@ -1069,6 +1072,28 @@ public:
const char *message, const char *message,
MYSQL_ERROR::enum_warning_level level, MYSQL_ERROR::enum_warning_level level,
THD *thd) = 0; 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: ...@@ -2210,6 +2235,9 @@ public:
thd_scheduler scheduler; thd_scheduler scheduler;
public: public:
inline Internal_error_handler *get_internal_handler()
{ return m_internal_handler; }
/** /**
Add an internal error handler to the thread execution context. Add an internal error handler to the thread execution context.
@param handler the exception handler to add @param handler the exception handler to add
......
...@@ -3883,7 +3883,9 @@ end_with_restore_list: ...@@ -3883,7 +3883,9 @@ end_with_restore_list:
res= mysql_routine_grant(thd, all_tables, res= mysql_routine_grant(thd, all_tables,
lex->type == TYPE_ENUM_PROCEDURE, lex->type == TYPE_ENUM_PROCEDURE,
lex->users_list, grants, lex->users_list, grants,
lex->sql_command == SQLCOM_REVOKE, 0); lex->sql_command == SQLCOM_REVOKE, TRUE);
if (!res)
my_ok(thd);
} }
else else
{ {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment