List:Commits« Previous MessageNext Message »
From:Davi Arnaut Date:May 25 2011 4:42pm
Subject:bzr commit into mysql-5.1 branch (davi:3629) Bug#12563279
View as plain text  
# At a local mysql-5.1 repository of davi

 3629 Davi Arnaut	2011-05-25
      Bug#12563279: REGRESSION IN HANDLING PRE-4.1 AUTHENTICATION PACKET
      
      The problem is that clients implementing the 4.0 version of the
      protocol (that is, mysql-4.0) do not null terminate a string
      at the end of the authentication packet. These clients denote
      the end of the string with the end of the packet.
      
      Although this goes against the documented (see MySQL Internals
      ClientServer Protocol wiki) description of the protocol,
      these old clients still need to be supported.
      
      The solution is to support the documented and actual behavior
      of the clients. If a client is using the pre-4.1 version of
      the protocol, the end of a string in the authentication packet
      can either be denoted with a null character or by the end of
      the packet. This restores backwards compatibility with old
      clients implementing either the documented or actual behavior.
     @ sql/sql_connect.cc
        Use different execution paths to read strings depending on the
        protocol being used. If version 4.0 of the protocol is used,
        end of string can be denoted with a NUL character or by the
        end of the packet.
        
        If there are not enough bytes left after the current position
        of the buffer to satisfy the current string, the string is
        considered to be empty. This is required because old clients
        do not send the password string field if the password is empty.

    modified:
      sql/sql_connect.cc
=== modified file 'sql/sql_connect.cc'
--- a/sql/sql_connect.cc	2011-05-16 20:04:01 +0000
+++ b/sql/sql_connect.cc	2011-05-25 16:42:24 +0000
@@ -653,14 +653,21 @@ bool init_new_connection_handler_thread(
 }
 
 #ifndef EMBEDDED_LIBRARY
+
+/** Get a string according to the protocol of the underlying buffer. */
+typedef char * (*get_proto_string_func_t) (char **, size_t *, size_t *);
+
 /**
-  Get a null character terminated string from a user-supplied buffer.
+  Get a string formatted according to the 4.1 version of the MySQL protocol.
 
-  @param buffer[in, out]    Pointer to the buffer to be scanned.
+  @param buffer[in, out]    Pointer to the user-supplied buffer to be scanned.
   @param max_bytes_available[in, out]  Limit the bytes to scan.
   @param string_length[out] The number of characters scanned not including
                             the null character.
 
+  @remark Strings are always null character terminated in this version of the
+          protocol.
+
   @remark The string_length does not include the terminating null character.
           However, after the call, the buffer is increased by string_length+1
           bytes, beyond the null character if there still available bytes to
@@ -671,9 +678,9 @@ bool init_new_connection_handler_thread(
 */
 
 static
-char *get_null_terminated_string(char **buffer,
-                                 size_t *max_bytes_available,
-                                 size_t *string_length)
+char *get_41_protocol_string(char **buffer,
+                             size_t *max_bytes_available,
+                             size_t *string_length)
 {
   char *str= (char *)memchr(*buffer, '\0', *max_bytes_available);
 
@@ -683,7 +690,60 @@ char *get_null_terminated_string(char **
   *string_length= (size_t)(str - *buffer);
   *max_bytes_available-= *string_length + 1;
   str= *buffer;
-  *buffer += *string_length + 1;  
+  *buffer += *string_length + 1;
+
+  return str;
+}
+
+
+/**
+  Get a string formatted according to the 4.0 version of the MySQL protocol.
+
+  @param buffer[in, out]    Pointer to the user-supplied buffer to be scanned.
+  @param max_bytes_available[in, out]  Limit the bytes to scan.
+  @param string_length[out] The number of characters scanned not including
+                            the null character.
+
+  @remark If there are not enough bytes left after the current position of
+          the buffer to satisfy the current string, the string is considered
+          to be empty and a pointer to empty_c_string is returned.
+
+  @remark A string at the end of the packet is not null terminated.
+
+  @return Pointer to beginning of the string scanned, or a pointer to a empty
+          string.
+*/
+static
+char *get_40_protocol_string(char **buffer,
+                             size_t *max_bytes_available,
+                             size_t *string_length)
+{
+  char *str;
+  size_t len;
+
+  /* No bytes to scan left, treat string as empty. */
+  if (!*max_bytes_available)
+  {
+    *string_length= 0;
+    return empty_c_string;
+  }
+
+  str= (char *) memchr(*buffer, '\0', *max_bytes_available);
+
+  /*
+    If the string was not null terminated by the client,
+    the remaining of the packet is the string. Otherwise,
+    advance the buffer past the end of the null terminated
+    string.
+  */
+  if (str == NULL)
+    len= *string_length= *max_bytes_available;
+  else
+    len= (*string_length= (size_t)(str - *buffer)) + 1;
+
+  str= *buffer;
+  *buffer+= len;
+  *max_bytes_available-= len;
 
   return str;
 }
@@ -695,7 +755,7 @@ char *get_null_terminated_string(char **
   @param buffer[in, out] The buffer to scan; updates position after scan.
   @param max_bytes_available[in, out] Limit the number of bytes to scan
   @param string_length[out] Number of characters scanned
-  
+
   @remark In case the length is zero, then the total size of the string is
     considered to be 1 byte; the size byte.
 
@@ -854,7 +914,7 @@ static int check_connection(THD *thd)
       part at the end of packet.
     */
     end= strmake(end, thd->scramble, SCRAMBLE_LENGTH_323) + 1;
-   
+
     int2store(end, server_capabilites);
     /* write server characteristics: up to 16 bytes allowed */
     end[2]=(char) default_charset_info->number;
@@ -952,7 +1012,20 @@ static int check_connection(THD *thd)
   if ((thd->client_capabilities & CLIENT_TRANSACTIONS) &&
       opt_using_transactions)
     net->return_status= &thd->server_status;
- 
+
+  /*
+    The 4.0 and 4.1 versions of the protocol differ on how strings
+    are terminated. In the 4.0 version, if a string is at the end
+    of the packet, the string is not null terminated. Do not assume
+    that the returned string is always null terminated.
+  */
+  get_proto_string_func_t get_string;
+
+  if (thd->client_capabilities & CLIENT_PROTOCOL_41)
+    get_string= get_41_protocol_string;
+  else
+    get_string= get_40_protocol_string;
+
   /*
     In order to safely scan a head for '\0' string terminators
     we must keep track of how many bytes remain in the allocated
@@ -961,8 +1034,7 @@ static int check_connection(THD *thd)
   size_t bytes_remaining_in_packet= pkt_len - (end - (char *)net->read_pos);
 
   size_t user_len;
-  char *user= get_null_terminated_string(&end, &bytes_remaining_in_packet,
-                                         &user_len);
+  char *user= get_string(&end, &bytes_remaining_in_packet, &user_len);
   if (user == NULL)
   {
     inc_host_errors(&thd->remote.sin_addr);
@@ -991,8 +1063,7 @@ static int check_connection(THD *thd)
     /*
       Old passwords are zero terminated strings.
     */
-    passwd= get_null_terminated_string(&end, &bytes_remaining_in_packet,
-                                       &passwd_len);
+    passwd= get_string(&end, &bytes_remaining_in_packet, &passwd_len);
   }
 
   if (passwd == NULL)
@@ -1007,8 +1078,7 @@ static int check_connection(THD *thd)
 
   if (thd->client_capabilities & CLIENT_CONNECT_WITH_DB)
   {
-    db= get_null_terminated_string(&end, &bytes_remaining_in_packet,
-                                   &db_len);
+    db= get_string(&end, &bytes_remaining_in_packet, &db_len);
     if (db == NULL)
     {
       inc_host_errors(&thd->remote.sin_addr);


Attachment: [text/bzr-bundle] bzr/davi.arnaut@oracle.com-20110525164224-dz30xn7p0w0yvnet.bundle
Thread
bzr commit into mysql-5.1 branch (davi:3629) Bug#12563279Davi Arnaut25 May