From 6217dd728a142253a1302067196154bf57319ca9 Mon Sep 17 00:00:00 2001 From: "cmiller@zippy.cornsilk.net" <> Date: Wed, 5 Mar 2008 09:33:32 -0500 Subject: [PATCH] Bug#33464: DROP FUNCTION caused a crash The cause of the crash is an assertion failure that we do not emit an error message (grant not found) and then return "ok". The assertion is valid, and we were ignoring the buggy behavior prior to the "Diagnostics" result-verification. Use an error handler to mutate innocuous missing-grant errors, when removing routines, into warnings. --- mysql-test/r/drop.result | 39 +++++++++++++++ mysql-test/t/drop.test | 62 +++++++++++++++++++++++ sql/sql_acl.cc | 105 +++++++++++++++++++++++++++++++-------- 3 files changed, 184 insertions(+), 22 deletions(-) diff --git a/mysql-test/r/drop.result b/mysql-test/r/drop.result index 71d6fcc7cd0..b4f6395819f 100644 --- a/mysql-test/r/drop.result +++ b/mysql-test/r/drop.result @@ -91,4 +91,43 @@ create table mysql_test.`#sql-347f_7` (f1 int); create table mysql_test.`#sql-347f_8` (f1 int); drop table mysql_test.`#sql-347f_8`; drop database mysql_test; +CREATE DATABASE dbbug33464; +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'; +END// +CREATE FUNCTION fn1() returns char(50) SQL SECURITY INVOKER +BEGIN +return 1; +END// +CREATE FUNCTION fn2() returns char(50) SQL SECURITY DEFINER +BEGIN +return 2; +END// +USE dbbug33464; + +root@localhost dbbug33464 +SELECT fn1(); +fn1() +1 +SELECT fn2(); +fn2() +2 +DROP USER 'userbug33464'@'localhost'; +DROP FUNCTION fn1; +Warnings: +Warning 1403 There is no such grant defined for user 'userbug33464' on host 'localhost' on routine 'fn1' +DROP FUNCTION fn2; +Warnings: +Warning 1403 There is no such grant defined for user 'userbug33464' on host 'localhost' on routine 'fn2' +DROP PROCEDURE sp3; +DROP USER 'userbug33464'@'localhost'; +use test; +DROP DATABASE dbbug33464; End of 5.1 tests diff --git a/mysql-test/t/drop.test b/mysql-test/t/drop.test index a79044436eb..3dd180f2d78 100644 --- a/mysql-test/t/drop.test +++ b/mysql-test/t/drop.test @@ -134,4 +134,66 @@ drop table mysql_test.`#sql-347f_8`; copy_file $MYSQLTEST_VARDIR/master-data/mysql_test/t1.frm $MYSQLTEST_VARDIR/master-data/mysql_test/#sql-347f_6.frm; drop database mysql_test; + +# +# Bug#33464: DROP FUNCTION caused a crash. +# +CREATE DATABASE dbbug33464; +CREATE USER 'userbug33464'@'localhost'; + +GRANT CREATE ROUTINE ON dbbug33464.* TO 'userbug33464'@'localhost'; + +--replace_result $MASTER_MYPORT MYSQL_PORT $MASTER_MYSOCK MYSQL_SOCK +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 + SELECT * from dbbug33464.t6 where t6.f2= 'xyz'; +END// +delimiter ;// + +delimiter //; +CREATE FUNCTION fn1() returns char(50) SQL SECURITY INVOKER +BEGIN + return 1; +END// +delimiter ;// + +delimiter //; +CREATE FUNCTION fn2() returns char(50) SQL SECURITY DEFINER +BEGIN + return 2; +END// +delimiter ;// + +disconnect connbug33464; + +# cleanup +connection default; +USE dbbug33464; +--source suite/funcs_1/include/show_connection.inc + +SELECT fn1(); +SELECT fn2(); + +--error 0, ER_CANNOT_USER +DROP USER 'userbug33464'@'localhost'; + +DROP FUNCTION fn1; +DROP FUNCTION fn2; +DROP PROCEDURE sp3; + +--error 0, ER_CANNOT_USER +DROP USER 'userbug33464'@'localhost'; + +use test; +DROP DATABASE dbbug33464; + --echo End of 5.1 tests diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc index 0d563ab9051..b9d72998933 100644 --- a/sql/sql_acl.cc +++ b/sql/sql_acl.cc @@ -2779,6 +2779,10 @@ table_error: } +/** + @retval 0 success + @retval -1 error +*/ static int replace_routine_table(THD *thd, GRANT_NAME *grant_name, TABLE *table, const LEX_USER &combo, const char *db, const char *routine_name, @@ -2800,14 +2804,11 @@ static int replace_routine_table(THD *thd, GRANT_NAME *grant_name, thd->security_ctx->host_or_ip, NullS); /* - The following should always succeed as new users are created before - this function is called! + New users are created before this function is called. + + There may be some cases where a routine's definer is removed but the + routine remains. */ - if (!find_acl_user(combo.host.str, combo.user.str, FALSE)) - { - my_error(ER_PASSWORD_NO_MATCH,MYF(0)); - DBUG_RETURN(-1); - } table->use_all_columns(); restore_record(table, s->default_values); // Get empty record @@ -3321,7 +3322,8 @@ bool mysql_routine_grant(THD *thd, TABLE_LIST *table_list, bool is_proc, } if (replace_routine_table(thd, grant_name, tables[1].table, *Str, - db_name, table_name, is_proc, rights, revoke_grant)) + db_name, table_name, is_proc, rights, + revoke_grant) != 0) { result= TRUE; continue; @@ -5949,11 +5951,11 @@ bool mysql_revoke_all(THD *thd, List &list) if (!strcmp(lex_user->user.str,user) && !strcmp(lex_user->host.str, host)) { - if (!replace_routine_table(thd,grant_proc,tables[4].table,*lex_user, + if (replace_routine_table(thd,grant_proc,tables[4].table,*lex_user, grant_proc->db, grant_proc->tname, is_proc, - ~(ulong)0, 1)) + ~(ulong)0, 1) == 0) { revoked= 1; continue; @@ -5979,17 +5981,73 @@ bool mysql_revoke_all(THD *thd, List &list) } -/* - Revoke privileges for all users on a stored procedure - SYNOPSIS - sp_revoke_privileges() + +/** + If the defining user for a routine does not exist, then the ACL lookup + code should raise two errors which we should intercept. We convert the more + descriptive error into a warning, and consume the other. + + If any other errors are raised, then we set a flag that should indicate + that there was some failure we should complain at a higher level. +*/ +class Silence_routine_definer_errors : public Internal_error_handler +{ +public: + Silence_routine_definer_errors() + : is_grave(FALSE) + {} + + virtual ~Silence_routine_definer_errors() + {} + + virtual bool handle_error(uint sql_errno, const char *message, + MYSQL_ERROR::enum_warning_level level, + THD *thd); + + bool has_errors() { return is_grave; } + +private: + bool is_grave; +}; + +bool +Silence_routine_definer_errors::handle_error(uint sql_errno, + const char *message, + MYSQL_ERROR::enum_warning_level level, + THD *thd) +{ + if (level == MYSQL_ERROR::WARN_LEVEL_ERROR) + { + switch (sql_errno) + { + case ER_NONEXISTING_PROC_GRANT: + /* Convert the error into a warning. */ + push_warning(thd, MYSQL_ERROR::WARN_LEVEL_WARN, sql_errno, message); + return TRUE; + default: + is_grave= TRUE; + } + } + + return FALSE; +} + + +/** + Revoke privileges for all users on a stored procedure. Use an error handler + that converts errors about missing grants into warnings. + + @param thd The current thread. + @param db DB of the stored procedure + @param name Name of the stored procedure - RETURN + @retval 0 OK. + @retval < 0 Error. Error message not yet sent. */ @@ -6000,11 +6058,15 @@ bool sp_revoke_privileges(THD *thd, const char *sp_db, const char *sp_name, int result; TABLE_LIST tables[GRANT_TABLES]; HASH *hash= is_proc ? &proc_priv_hash : &func_priv_hash; + Silence_routine_definer_errors error_handler; DBUG_ENTER("sp_revoke_privileges"); if ((result= open_grant_tables(thd, tables))) DBUG_RETURN(result != 1); + /* Be sure to pop this before exiting this scope! */ + thd->push_internal_handler(&error_handler); + rw_wrlock(&LOCK_grant); VOID(pthread_mutex_lock(&acl_cache->lock)); @@ -6031,14 +6093,14 @@ bool sp_revoke_privileges(THD *thd, const char *sp_db, const char *sp_name, grant_proc->host.hostname : (char*)""; lex_user.host.length= grant_proc->host.hostname ? strlen(grant_proc->host.hostname) : 0; - if (!replace_routine_table(thd,grant_proc,tables[4].table,lex_user, - grant_proc->db, grant_proc->tname, - is_proc, ~(ulong)0, 1)) + + if (replace_routine_table(thd,grant_proc,tables[4].table,lex_user, + grant_proc->db, grant_proc->tname, + is_proc, ~(ulong)0, 1) == 0) { revoked= 1; continue; } - result= -1; // Something went wrong } counter++; } @@ -6048,10 +6110,9 @@ bool sp_revoke_privileges(THD *thd, const char *sp_db, const char *sp_name, rw_unlock(&LOCK_grant); close_thread_tables(thd); - if (result) - my_message(ER_REVOKE_GRANTS, ER(ER_REVOKE_GRANTS), MYF(0)); + thd->pop_internal_handler(); - DBUG_RETURN(result); + DBUG_RETURN(error_handler.has_errors()); }