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

(Pushing for Azundris)
      
We allow security-contexts with NULL users (for
system-threads and for unauthenticated users).
If a non-SUPER-user tried to KILL such a thread,
we tried to compare the user-fields to see whether
they owned that thread. Comparing against NULL was
not a good idea.
      
If KILLer does not have SUPER-privilege, we
specifically check whether both KILLer and KILLee
have a non-NULL user before testing for string-
equality. If either is NULL, we reject the KILL.

mysql-test/r/rpl_temporary.result

@@ -4,6 +4,24 @@ 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;

mysql-test/t/rpl_temporary.test

@@ -3,6 +3,42 @@ 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

sql/sql_parse.cc

@@ -7386,8 +7386,27 @@ void kill_one_thread(THD *thd, ulong id, bool only_kill_query)
   VOID(pthread_mutex_unlock(&LOCK_thread_count));
   if (tmp)
   {
+
+    /*
+      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.
+
+      tmp..user will be NULL for system threads (cf Bug#43748).
+      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.
+
+      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) ||
-	!strcmp(thd->security_ctx->user, tmp->security_ctx->user))
+        ((thd->security_ctx->user != NULL) &&
+         (tmp->security_ctx->user != NULL) &&
+         !strcmp(thd->security_ctx->user, tmp->security_ctx->user)))
     {
       tmp->awake(only_kill_query ? THD::KILL_QUERY : THD::KILL_CONNECTION);
       error=0;