Bug #92954 COM_CHANGE_USER command failed when client has CLIENT_CONNECT_ATTRS capability
Submitted: 26 Oct 2018 7:40 Modified: 15 Nov 2018 13:36
Reporter: andy zhang Email Updates:
Status: Verified Impact on me:
None 
Category:MySQL Server: C API (client library) Severity:S3 (Non-critical)
Version:5.6 and above OS:Any
Assigned to: CPU Architecture:Any
Tags: Com_change_user

[26 Oct 2018 7:40] andy zhang
Description:
parse_com_change_user_packet  did't position the ptr correctly before invoking read_client_connect_attrs while handling  COM_CHANGE_USER command from client with 
CLIENT_CONNECT_ATTRS capability.

<=======
  if ((mpvio->client_capabilities & CLIENT_CONNECT_ATTRS) &&
      read_client_connect_attrs(mpvio->thd, &ptr, &bytes_remaining_in_packet,
                                mpvio->charset_adapter->charset()))
    return packet_error;
========>

How to repeat:
Issue COM_CHANGE_USER from client with CLIENT_CONNECT_ATTRS capability

Suggested fix:
Advance the ptr in parse_com_change_user_packet as below

--- a/sql/sql_acl.cc
+++ b/sql/sql_acl.cc
@@ -10524,6 +10524,8 @@ static bool parse_com_change_user_packet(MPVIO_EXT *mpvio, uint packet_length)
   {
     if (mpvio->charset_adapter->init_client_charset(uint2korr(ptr)))
       DBUG_RETURN(1);
+
+    ptr= ptr + 2; // skip 2-bytes character set number
   }
   else
   {
@@ -10569,14 +10571,16 @@ static bool parse_com_change_user_packet(MPVIO_EXT *mpvio, uint packet_length)
     DBUG_RETURN(1);
 
   char *client_plugin;
+
   if (mpvio->client_capabilities & CLIENT_PLUGIN_AUTH)
   {
-    client_plugin= ptr + 2;
+    client_plugin= ptr;
     if (client_plugin >= end)
     {
       my_message(ER_UNKNOWN_COM_ERROR, ER(ER_UNKNOWN_COM_ERROR), MYF(0));
       DBUG_RETURN(1);
     }
+    ptr= ptr + strlen(client_plugin) + 1; //move the poitner across the '\0' ended client_plugin
   }
[13 Nov 2018 13:20] Sinisa Milivojevic
Hi,

I can confirm that your analysis is correct and that character set 2-byte code is not skipped immediately. However, the pointer `ptr` is NOT used between the command for getting the character set and the assignment of the `client_plugin` pointer.

Simply, pointer `ptr` is NOT used at all between the two lines.

Hence, I do not see how can this be a bug ???
[15 Nov 2018 13:19] andy zhang
Sinisa,

Please look ahead for some more lines of code. Ptr will be referenced as the start of attributes string. The reason the problem was never exposed is that no case qualified 
mpvio->client_capabilities & CLIENT_CONNECT_ATTRS. But in our environment, our application entered the path.

  size_t bytes_remaining_in_packet= end - ptr;

  if ((mpvio->client_capabilities & CLIENT_CONNECT_ATTRS) &&
      read_client_connect_attrs(&ptr, &bytes_remaining_in_packet,
                                mpvio->charset_adapter->charset()))
    return packet_error;
[15 Nov 2018 13:36] Sinisa Milivojevic
Hi,

Unfortunately, I have to admit that this is a bug. In the function that you mention, `ptr` is supposed to point to the length and not to the character set code.

Verified as reported.