Bug#43748: crash when non-super user tries to kill the replication threads

Fine-tuning. Broke out comparison into method by
suggestion of Davi. Clarified comments. Reverting
test-case which I find too brittle; proper test
case in 5.1+.

5 files changed, 16 insertions, 63 deletions

diff --git a/mysql-test/r/rpl_temporary.result b/mysql-test/r/rpl_temporary.result
index 5eefced7564..15c069ab68d 100644
--- a/mysql-test/r/rpl_temporary.result
+++ b/mysql-test/r/rpl_temporary.result
@@ -4,24 +4,6 @@ reset master;
 reset slave;
 drop table if exists t1,t2,t3,t4,t5,t6,t7,t8,t9;
 start slave;
-FLUSH PRIVILEGES;
-drop table if exists t999;
-create temporary table t999(
-id int,
-user char(255),
-host char(255),
-db char(255),
-Command char(255),
-time int,
-State char(255),
-info char(255)
-);
-LOAD DATA INFILE "./tmp/bl_dump_thread_id" into table t999;
-drop table t999;
-GRANT USAGE ON *.* TO user43748@localhost;
-KILL `select id from information_schema.processlist where command='Binlog Dump'`;
-ERROR HY000: You are not owner of thread `select id from information_schema.processlist where command='Binlog Dump'`
-DROP USER user43748@localhost;
 reset master;
 SET @save_select_limit=@@session.sql_select_limit;
 SET @@session.sql_select_limit=10, @@session.pseudo_thread_id=100;
diff --git a/mysql-test/t/rpl_temporary.test b/mysql-test/t/rpl_temporary.test
index 366fdf00c56..516f3a026c9 100644
--- a/mysql-test/t/rpl_temporary.test
+++ b/mysql-test/t/rpl_temporary.test
@@ -3,42 +3,6 @@ source include/add_anonymous_users.inc;
 
 source include/master-slave.inc;
 
-#
-# Bug#43748: crash when non-super user tries to kill the replication threads
-#
-
---connection master
-save_master_pos;
-
---connection slave
-sync_with_master;
-
---connection slave
-FLUSH PRIVILEGES;
-
-# in 5.0, we need to do some hocus pocus to get a system-thread ID (-> $id)
---source include/get_binlog_dump_thread_id.inc
-
-# make a non-privileged user on slave. try to KILL system-thread as her.
-GRANT USAGE ON *.* TO user43748@localhost;
-
---connect (mysqltest_2_con,localhost,user43748,,test,$SLAVE_MYPORT,)
---connection mysqltest_2_con
-
---replace_result $id "`select id from information_schema.processlist where command='Binlog Dump'`"
---error ER_KILL_DENIED_ERROR
-eval KILL $id;
-
---disconnect mysqltest_2_con
-
---connection slave
-
-DROP USER user43748@localhost;
-
---connection master
-
-
-
 # Clean up old slave's binlogs.
 # The slave is started with --log-slave-updates
 # and this test does SHOW BINLOG EVENTS on the slave's
diff --git a/sql/sql_class.cc b/sql/sql_class.cc
index 74bc669a049..aa796d6acbd 100644
--- a/sql/sql_class.cc
+++ b/sql/sql_class.cc
@@ -2144,6 +2144,13 @@ void Security_context::skip_grants()
 }
 
 
+bool Security_context::user_matches(Security_context *them)
+{
+  return ((user != NULL) && (them->user != NULL) &&
+          !strcmp(user, them->user));
+}
+
+
 /****************************************************************************
   Handling of open and locked tables states.
 
diff --git a/sql/sql_class.h b/sql/sql_class.h
index 3e3dfcd08fa..47dbac0f17b 100644
--- a/sql/sql_class.h
+++ b/sql/sql_class.h
@@ -985,6 +985,7 @@ public:
   {
     return (*priv_host ? priv_host : (char *)"%");
   }
+  bool user_matches(Security_context *);
 };
 
 
diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc
index 33adcfe3342..c2d789b30b5 100644
--- a/sql/sql_parse.cc
+++ b/sql/sql_parse.cc
@@ -7391,22 +7391,21 @@ void kill_one_thread(THD *thd, ulong id, bool only_kill_query)
       If we're SUPER, we can KILL anything, including system-threads.
       No further checks.
 
-      thd..user could in theory be NULL while we're still in
-      "unauthenticated" state. This is more a theoretical case.
+      KILLer: thd->security_ctx->user could in theory be NULL while
+      we're still in "unauthenticated" state. This is a theoretical
+      case (the code suggests this could happen, so we play it safe).
 
-      tmp..user will be NULL for system threads (cf Bug#43748).
+      KILLee: tmp->security_ctx->user will be NULL for system threads.
       We need to check so Jane Random User doesn't crash the server
-      when trying to kill a) system threads or b) unauthenticated
-      users' threads.
+      when trying to kill a) system threads or b) unauthenticated users'
+      threads (Bug#43748).
 
-      If user of both killer and killee are non-null, proceed with
+      If user of both killer and killee are non-NULL, proceed with
       slayage if both are string-equal.
     */
 
     if ((thd->security_ctx->master_access & SUPER_ACL) ||
-        ((thd->security_ctx->user != NULL) &&
-         (tmp->security_ctx->user != NULL) &&
-         !strcmp(thd->security_ctx->user, tmp->security_ctx->user)))
+        thd->security_ctx->user_matches(tmp->security_ctx))
     {
       tmp->awake(only_kill_query ? THD::KILL_QUERY : THD::KILL_CONNECTION);
       error=0;