From 5b497b163aff1ba23af4f97528be832af55c3c21 Mon Sep 17 00:00:00 2001 From: unknown Date: Fri, 10 Feb 2006 15:02:57 +0100 Subject: [PATCH 1/2] fix for bug #17289 Events: missing privilege check for drop database Events were executed with all privileges possible on planet Earth :( WL#1034 mysql-test/r/events.result: update test results mysql-test/t/events.test: test for bug#17289 Events: missing privilege check for drop database sql/event.h: -add two new methods to event_timed to change and restore the security context sql/event_executor.cc: - move code regarding privilieges checking to event_timed::execute() - add a new function evex_print_warnings() which prints the notes/warnings/errors to the console (easily capturable with logs-into-tables) so one can see what has happened if there was an error of some sort! sql/event_timed.cc: - fix documentation - add a new error code -99, EVENT was revoked from the user on the DB - set_sec_ctx, execute, restore_sex_ctx sql/sql_error.cc: - make warning_level_names public to be used in event_executor.cc - change from 2 arrays to a LEX_STRING array --- mysql-test/r/events.result | 27 +++++++++++- mysql-test/t/events.test | 33 +++++++++++++++ sql/event.h | 6 +++ sql/event_executor.cc | 86 ++++++++++++++++++++++++++++---------- sql/event_timed.cc | 84 +++++++++++++++++++++++++++++++++++-- sql/sql_error.cc | 13 ++++-- 6 files changed, 219 insertions(+), 30 deletions(-) diff --git a/mysql-test/r/events.result b/mysql-test/r/events.result index b5b1b76f1a8..9b49ea24f26 100644 --- a/mysql-test/r/events.result +++ b/mysql-test/r/events.result @@ -1,5 +1,28 @@ create database if not exists events_test; use events_test; +CREATE USER pauline@localhost; +CREATE DATABASE db_x; +GRANT EVENT ON db_x.* TO pauline@localhost; +USE db_x; +CREATE TABLE x_table(a int); +CREATE EVENT e_x1 ON SCHEDULE EVERY 1 SECOND DO DROP DATABASE db_x; +CREATE EVENT e_x2 ON SCHEDULE EVERY 1 SECOND DO DROP TABLE x_table; +SHOW DATABASES LIKE 'db_x'; +Database (db_x) +db_x +SET GLOBAL event_scheduler=1; +SHOW DATABASES LIKE 'db_x'; +Database (db_x) +db_x +SHOW TABLES FROM db_x; +Tables_in_db_x +x_table +SET GLOBAL event_scheduler=0; +DROP EVENT e_x1; +DROP EVENT e_x2; +DROP DATABASE db_x; +DROP USER pauline@localhost; +USE events_test; drop event if exists event1; Warnings: Note 1305 Event event1 does not exist @@ -166,7 +189,7 @@ show processlist; Id User Host db Command Time State Info # root localhost events_test Query # NULL show processlist # event_scheduler NULL Connect # Sleeping NULL -# root events_test Connect # User lock select get_lock("test_lock2", 20) +# root localhost events_test Connect # User lock select get_lock("test_lock2", 20) "Release the mutex, the event worker should finish." select release_lock("test_lock2"); release_lock("test_lock2") @@ -184,6 +207,8 @@ set global event_scheduler=0; show processlist; Id User Host db Command Time State Info # root localhost events_test Query # NULL show processlist +# event_scheduler NULL Connect # Sleeping NULL +# root localhost events_test Connect # User lock select get_lock("test_lock2_1", 20) "Release the lock so the child process should finish. Hence the scheduler also" select release_lock("test_lock2_1"); release_lock("test_lock2_1") diff --git a/mysql-test/t/events.test b/mysql-test/t/events.test index aa759d5ca6f..5a58d29e0ff 100644 --- a/mysql-test/t/events.test +++ b/mysql-test/t/events.test @@ -1,5 +1,38 @@ create database if not exists events_test; use events_test; + +# +# START: BUG #17289 Events: missing privilege check for drop database +# +CREATE USER pauline@localhost; +CREATE DATABASE db_x; +GRANT EVENT ON db_x.* TO pauline@localhost; +USE db_x; +CREATE TABLE x_table(a int); +connect (priv_conn,localhost,pauline,,db_x); +CREATE EVENT e_x1 ON SCHEDULE EVERY 1 SECOND DO DROP DATABASE db_x; +CREATE EVENT e_x2 ON SCHEDULE EVERY 1 SECOND DO DROP TABLE x_table; +connection default; +SHOW DATABASES LIKE 'db_x'; +SET GLOBAL event_scheduler=1; +--sleep 2 +SHOW DATABASES LIKE 'db_x'; +SHOW TABLES FROM db_x; +SET GLOBAL event_scheduler=0; +--sleep 1 +connection priv_conn; +DROP EVENT e_x1; +DROP EVENT e_x2; +disconnect priv_conn; +connection default; +DROP DATABASE db_x; +DROP USER pauline@localhost; +USE events_test; +--sleep 1 +# +# END: BUG #17289 Events: missing privilege check for drop database +# + drop event if exists event1; create event event1 on schedule every 15 minute starts now() ends date_add(now(), interval 5 hour) DO begin end; alter event event1 rename to event2 enable; diff --git a/sql/event.h b/sql/event.h index 1fe5c8e5713..bff8c1d3d8e 100644 --- a/sql/event.h +++ b/sql/event.h @@ -203,6 +203,12 @@ public: delete sphead; sphead= 0; } +protected: + bool + change_security_context(THD *thd, Security_context **backup); + + void + restore_security_context(THD *thd, Security_context *backup); }; diff --git a/sql/event_executor.cc b/sql/event_executor.cc index d06e3e57a1e..d073b4a91bc 100644 --- a/sql/event_executor.cc +++ b/sql/event_executor.cc @@ -49,7 +49,8 @@ static uint workers_count; static int evex_load_events_from_db(THD *thd); - +bool +evex_print_warnings(THD *thd, event_timed *et); /* TODO Andrey: Check for command line option whether to start @@ -135,7 +136,8 @@ init_event_thread(THD* thd) { DBUG_ENTER("init_event_thread"); thd->client_capabilities= 0; - thd->security_ctx->skip_grants(); + thd->security_ctx->master_access= 0; + thd->security_ctx->db_access= 0; thd->security_ctx->host= (char*)my_localhost; my_net_init(&thd->net, 0); thd->net.read_timeout = slave_net_timeout; @@ -204,7 +206,7 @@ event_executor_main(void *arg) if (init_event_thread(thd)) goto err; - + thd->security_ctx->skip_grants(); // make this thread invisible it has no vio -> show processlist won't see thd->system_thread= 1; @@ -481,21 +483,8 @@ event_executor_worker(void *event_void) thd= current_thd; #endif - // thd->security_ctx->priv_host is char[MAX_HOSTNAME] - - strxnmov(thd->security_ctx->priv_host, sizeof(thd->security_ctx->priv_host), - event->definer_host.str, NullS); - - thd->security_ctx->user= thd->security_ctx->priv_user= - my_strdup(event->definer_user.str, MYF(0)); - - thd->db= event->dbname.str; - if (!check_access(thd, EVENT_ACL, event->dbname.str, 0, 0, 0, - is_schema_db(event->dbname.str))) { int ret; - DBUG_PRINT("info", (" EVEX EXECUTING event %s.%s [EXPR:%d]", - event->dbname.str, event->name.str,(int) event->expression)); sql_print_information(" EVEX EXECUTING event %s.%s [EXPR:%d]", event->dbname.str, event->name.str,(int) event->expression); @@ -507,10 +496,7 @@ event_executor_worker(void *event_void) if (ret == EVEX_COMPILE_ERROR) sql_print_information(" EVEX COMPILE ERROR for event %s.%s", event->dbname.str, event->name.str); - - DBUG_PRINT("info", (" EVEX EXECUTED event %s.%s [EXPR:%d]. RetCode=%d", - event->dbname.str, event->name.str, - (int) event->expression, ret)); + evex_print_warnings(thd, event); } if ((event->flags & EVENT_EXEC_NO_MORE) || event->status==MYSQL_EVENT_DISABLED) { @@ -521,7 +507,6 @@ event_executor_worker(void *event_void) delete event; } - thd->db= 0; err: VOID(pthread_mutex_lock(&LOCK_thread_count)); @@ -666,3 +651,62 @@ bool sys_var_event_executor::update(THD *thd, set_var *var) DBUG_RETURN(0); } +extern LEX_STRING warning_level_names[]; + +/* + Prints the stack of infos, warnings, errors from thd to + the console so it can be fetched by the logs-into-tables and + checked later. + + Synopsis + evex_print_warnings + thd - thread used during the execution of the event + et - the event itself + + Returns + 0 - OK (always) + +*/ + +bool +evex_print_warnings(THD *thd, event_timed *et) +{ + MYSQL_ERROR *err; + DBUG_ENTER("evex_show_warnings"); + char msg_buf[1024]; + char prefix_buf[512]; + String prefix(prefix_buf, sizeof(prefix_buf), system_charset_info); + prefix.length(0); + + List_iterator_fast it(thd->warn_list); + while ((err= it++)) + { + String err_msg(msg_buf, sizeof(msg_buf), system_charset_info); + err_msg.length(0);// set it to 0 or we start adding at the end + if (!prefix.length()) + { + prefix.append("SCHEDULER: ["); + + append_identifier(thd,&prefix,et->definer_user.str,et->definer_user.length); + prefix.append('@'); + append_identifier(thd,&prefix,et->definer_host.str,et->definer_host.length); + prefix.append("][", 2); + append_identifier(thd,&prefix, et->dbname.str, et->dbname.length); + prefix.append('.'); + append_identifier(thd,&prefix, et->name.str, et->name.length); + prefix.append("] ", 2); + } + + err_msg.append(prefix); + err_msg.append('['); + err_msg.append(warning_level_names[err->level].str, + warning_level_names[err->level].length, system_charset_info); + err_msg.append("] ["); + err_msg.append(err->msg, strlen(err->msg), system_charset_info); + err_msg.append("]"); + sql_print_information("%*s", err_msg.length(), err_msg.c_ptr()); + } + + + DBUG_RETURN(FALSE); +} diff --git a/sql/event_timed.cc b/sql/event_timed.cc index e585f6252ca..62fcf4cbf28 100644 --- a/sql/event_timed.cc +++ b/sql/event_timed.cc @@ -982,12 +982,13 @@ event_timed::get_show_create_event(THD *thd, uint32 *length) Executes the event (the underlying sp_head object); SYNOPSIS - evex_fill_row() + event_timed::execute() thd THD mem_root If != NULL use it to compile the event on it Returns 0 - success + -99 - No access to the database. -100 - event in execution (parallel execution is impossible) others - retcodes of sp_head::execute_procedure() @@ -996,10 +997,12 @@ event_timed::get_show_create_event(THD *thd, uint32 *length) int event_timed::execute(THD *thd, MEM_ROOT *mem_root) { - List empty_item_list; + Security_context *save_ctx; int ret= 0; DBUG_ENTER("event_timed::execute"); + DBUG_PRINT("info", (" EVEX EXECUTING event %s.%s [EXPR:%d]", + dbname.str, name.str, (int) expression)); VOID(pthread_mutex_lock(&this->LOCK_running)); if (running) @@ -1011,12 +1014,35 @@ event_timed::execute(THD *thd, MEM_ROOT *mem_root) VOID(pthread_mutex_unlock(&this->LOCK_running)); // TODO Andrey : make this as member variable and delete in destructor - empty_item_list.empty(); if (!sphead && (ret= compile(thd, mem_root))) goto done; - ret= sphead->execute_procedure(thd, &empty_item_list); + thd->db= dbname.str; + thd->db_length= dbname.length; + + DBUG_PRINT("info", ("master_access=%d db_access=%d", + thd->security_ctx->master_access, thd->security_ctx->db_access)); + change_security_context(thd, &save_ctx); + DBUG_PRINT("info", ("master_access=%d db_access=%d", + thd->security_ctx->master_access, thd->security_ctx->db_access)); +// if (mysql_change_db(thd, dbname.str, 0)) + if (!check_access(thd, EVENT_ACL,dbname.str, 0, 0, 0,is_schema_db(dbname.str))) + { + List empty_item_list; + empty_item_list.empty(); + ret= sphead->execute_procedure(thd, &empty_item_list); + } + else + { + DBUG_PRINT("error", ("%s@%s has no rights on %s", definer_user.str, + definer_host.str, dbname.str)); + ret= -99; + } + restore_security_context(thd, save_ctx); + DBUG_PRINT("info", ("master_access=%d db_access=%d", + thd->security_ctx->master_access, thd->security_ctx->db_access)); + thd->db= 0; VOID(pthread_mutex_lock(&this->LOCK_running)); running= false; @@ -1029,11 +1055,61 @@ done: delete sphead; sphead= 0; } + DBUG_PRINT("info", (" EVEX EXECUTED event %s.%s [EXPR:%d]. RetCode=%d", + dbname.str, name.str, (int) expression, ret)); DBUG_RETURN(ret); } +/* + Switches the security context + Synopsis + event_timed::change_security_context() + thd - thread + backup - where to store the old context + + RETURN + 0 - OK + 1 - Error (generates error too) +*/ +bool +event_timed::change_security_context(THD *thd, Security_context **backup) +{ + DBUG_ENTER("event_timed::change_security_context"); + DBUG_PRINT("info",("%s@%s@%s",definer_user.str,definer_host.str, dbname.str)); + *backup= 0; + if (acl_getroot_no_password(&sphead->m_security_ctx, definer_user.str, + definer_host.str, definer_host.str, dbname.str)) + { + my_error(ER_NO_SUCH_USER, MYF(0), definer_user.str, definer_host.str); + DBUG_RETURN(TRUE); + } + *backup= thd->security_ctx; + thd->security_ctx= &sphead->m_security_ctx; + + DBUG_RETURN(FALSE); +} + + +/* + Restores the security context + Synopsis + event_timed::restore_security_context() + thd - thread + backup - switch to this context + */ + +void +event_timed::restore_security_context(THD *thd, Security_context *backup) +{ + DBUG_ENTER("event_timed::change_security_context"); + if (backup) + thd->security_ctx= backup; + DBUG_VOID_RETURN; +} + + /* Returns 0 - Success diff --git a/sql/sql_error.cc b/sql/sql_error.cc index 191a6e0a1fd..74df3b68a0d 100644 --- a/sql/sql_error.cc +++ b/sql/sql_error.cc @@ -211,8 +211,13 @@ void push_warning_printf(THD *thd, MYSQL_ERROR::enum_warning_level level, TRUE Error sending data to client */ -static const char *warning_level_names[]= {"Note", "Warning", "Error", "?"}; -static int warning_level_length[]= { 4, 7, 5, 1 }; +LEX_STRING warning_level_names[]= +{ + {(char*) STRING_WITH_LEN("Note")}, + {(char*) STRING_WITH_LEN("Warning")}, + {(char*) STRING_WITH_LEN("Error")}, + {(char*) STRING_WITH_LEN("?")} +}; bool mysqld_show_warnings(THD *thd, ulong levels_to_show) { @@ -246,8 +251,8 @@ bool mysqld_show_warnings(THD *thd, ulong levels_to_show) if (idx > unit->select_limit_cnt) break; protocol->prepare_for_resend(); - protocol->store(warning_level_names[err->level], - warning_level_length[err->level], system_charset_info); + protocol->store(warning_level_names[err->level].str, + warning_level_names[err->level].length, system_charset_info); protocol->store((uint32) err->code); protocol->store(err->msg, strlen(err->msg), system_charset_info); if (protocol->write()) From 77970a98d1d99fca176ebae085cb915af3a203a3 Mon Sep 17 00:00:00 2001 From: unknown Date: Tue, 14 Feb 2006 17:51:22 +0100 Subject: [PATCH 2/2] post-merge fixes sql/event_executor.cc: fix for bug#17289 (Events: missing privilege check for drop database) WL#1034 post-merge fixes --- sql/event_executor.cc | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/sql/event_executor.cc b/sql/event_executor.cc index d073b4a91bc..970814f291b 100644 --- a/sql/event_executor.cc +++ b/sql/event_executor.cc @@ -206,8 +206,11 @@ event_executor_main(void *arg) if (init_event_thread(thd)) goto err; - thd->security_ctx->skip_grants(); - // make this thread invisible it has no vio -> show processlist won't see + + /* + make this thread visible it has no vio -> show processlist won't see it + unless it's marked as system thread + */ thd->system_thread= 1; VOID(pthread_mutex_lock(&LOCK_thread_count)); @@ -653,6 +656,14 @@ bool sys_var_event_executor::update(THD *thd, set_var *var) extern LEX_STRING warning_level_names[]; +typedef void (*sql_print_xxx_func)(const char *format, ...); +static sql_print_xxx_func sql_print_xxx_handlers[3] = +{ + sql_print_information, + sql_print_warning, + sql_print_error +}; + /* Prints the stack of infos, warnings, errors from thd to the console so it can be fetched by the logs-into-tables and @@ -704,7 +715,9 @@ evex_print_warnings(THD *thd, event_timed *et) err_msg.append("] ["); err_msg.append(err->msg, strlen(err->msg), system_charset_info); err_msg.append("]"); - sql_print_information("%*s", err_msg.length(), err_msg.c_ptr()); + DBUG_ASSERT(err->level < 3); + (sql_print_xxx_handlers[err->level])("%*s", err_msg.length(), err_msg.c_ptr()); +// sql_print_information("%*s", err_msg.length(), err_msg.c_ptr()); }